On Tue Mar 7 00:49:58 2023 +0000, Francisco Casas wrote:
Well, perhaps that alone is not a big thing as I thought in the first place. I merged both things in the same struct, I pushed the result for your inspection. There are benefits: * Removing the `offset_reservation` fields from the parse symbols structs (`struct parse_parameter`, `struct parse_colon_attribute`, and `struct parse_variable_def`). * Removing the additional argument from `hlsl_new_var()`. There are downsides: * An extra refactoring patch for renaming the current fields in `struct hlsl_reg_reservation` was required. * In the parser, both the `register_opt` and the `packoffset_opt` symbols have to be `struct hlsl_reg_reservation`, so they both have to left the fields that don't correspond to them as zero. * `struct hlsl_buffer` can have a register reservation but not an offset reservation, which makes its `reservation` field a little unintuitive. * It is tempting to rename `reg_reservation` to just `reservation` in `struct hlsl_ir_var` and the parse symbols structs now, since they no longer contain just the reservation of the register, and `reg_reservation.offset_index` sounds a little odd. That would be a lot of refactoring though. If you don't like how this change ended up, I can revert it. I made a backup branch. I think it's an improvement, yeah. Not that it's a big deal to begin with, of course, but the extra patch to rename doesn't faze me if the code is improved as a result.
`struct hlsl_buffer` can have a register reservation but not an offset reservation, which makes its `reservation` field a little unintuitive.
True, but I'm not that bothered by that, it's not the only instance of fields that don't all apply. E.g. the writemask of hlsl_reg is irrelevant for buffers, or for large variables. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/106#note_26281