huixie90 wrote:
> Bitfield load and store operations should be done using the same offset/size
> we normally use to access the bitfield; unconditionally using byte load/store
> operations will impair optimizations/performance. I guess this might not be
> possible when unions are involved, but it shouldn't be that hard for the
> non-union cases.
>
> The format of builtin-clear-padding-codegen.cpp seems mostly fine, but
> consider using update_cc_test_checks.py to automate writing the CHECK lines.
> Please add a couple tests for empty classes and unions.
>
> A few comments in the code outlining how the recursion and the interval
> representation work would be helpful.
Thanks very much for your review. and really sorry it took me more than a year
to come back to this.
> unconditionally using byte load/store operations will impair
> optimizations/performance.
If you still remember this comment, is it referring to the final "clearing
padding step", where I zeroing bytes-by-bytes? If so, apologies for not being
familiar with this, what would be the best way of achieving it? So my current
approach is
- Visit recursively to figure out all the bits ranges that data occupied
- figure out all the holes (padding)
- generate storing zero bytes-by-bytes for the wholes bytes and bits
on the last step, for non-bitfield, i was basically doing
```cpp
Address ElementAddr(Element, CGF.Int8Ty, CharUnits::One());
CGF.Builder.CreateStore(Zero, ElementAddr);
```
for bitfield, i was basically doing
```cpp
uint8_t mask = ((1 << EndBit) - 1) & ~((1 << StartBit) - 1);
auto *MaskValue = ConstantInt::get(CGF.Int8Ty, mask);
auto *NewValue = CGF.Builder.CreateAnd(Value, MaskValue);
```
This might not be the most optimised way of doing this. however, I am not
familiar with this part of the code what would be the alternative.
> Bitfield load and store operations should be done using the same offset/size
> we normally use to access the bitfield;
Hmm, the puzzle I have is that I am not loading/storing the BitField
themselves, but the paddings around them, which may or may not be occupied by
other stuff.
> The format of builtin-clear-padding-codegen.cpp seems mostly fine, but
> consider using update_cc_test_checks.py to automate writing the CHECK lines.
> Please add a couple tests for empty classes and unions.
Absolutely, thanks for pointing out to update_cc_test_checks.py . I was mainly
testing using our libc++ test suites and was not sure how to automatically
generate these IR codegen tests. will update the test to cover all the cases.
https://github.com/llvm/llvm-project/pull/75371
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
