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.