Lancern wrote: Thanks for the elaboration!
I agree that it indeed looks chaotic if we keep empty `[[no_unique_address]]` members in CIR structs. And it could be horrible for LowerToLLVM authors to keep track of the adjustments of GEP offsets. My concern here is that we now have two representations for constant data member pointers, one based on the member index (`#cir.data_member`, already present), and the other based on the offset (`#cir.data_member_offset` introduced in this patch). And it's obvious that the former could be fully replaced by the latter, i.e. we could just lower all constant data member pointers to `#cir.data_member_offset` and then remove `#cir.data_member`. Will this happen? Furthermore, I hope we could agree that "member index" is a higher-level representation compared to "member offsets", and it's preferable to keep higher-level information in CIR, at least in the early stages. If we're going to unite `#cir.data_member` and `#cir.data_member_offset`, I hope we could have an optional attribute in `#cir.data_member_offset` that gives the indices of the referenced member. Here are some long-term thoughts I have on this that may not be immediately feasible in our current status (so I'm folding it in a `<detail>` block). <details> Keeping empty members in CIR looks chaotic because the input source code itself carries a ton of information. By removing these empty members we're throwing away high-level information that could be valuable for CIR consumers. So I think it's preferable to keep empty members and keep `#cir.data_member` as-is. I'm actually expecting that in the long term, the ABI lowering pass (instead of the LowerToLLVM pass) could: - Lower the high-level CIR struct (which contains empty members) to an ABI CIR struct (which lacks empty members and aligns to LLVMIR struct). - Lower all `#cir.data_member` attributes to `#cir.data_member_offset` (or maybe directly to `#cir.int`), by querying the layout information of the struct. </details> https://github.com/llvm/llvm-project/pull/208777 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
