[clang] [clang] Better bitfield access units (PR #65742)

[John McCall via cfe-commits]

rjmccall wrote:

> @rjmccall thanks for your comments. Here's a reimplementation using a single loop -- the downside is that we may end up repeating the initial grouping scan, if we search more than 1 Access Unit ahead and fail to find a 'good' access unit. This update changes the algorithm slightly, as I realized that `struct A { short a: 16; char b:8; int c;};` and `struct B { char b: 8; short a:16; int c;};` should have the same (32-bit) access unit. Originally only `A` got that.

Thanks.  Sorry about the delay, I'll try to take a look in the next day.

> 1. I noticed that several `CGRecordLowering` member fns could either be `const` or `static` -- would you like that as a separate NFC PR?

Yes, please.

> 2. I have corrected the error about merging across zero-sized members.
> 3. It may not be obvious from GH, but this PR breaks down to 3 (or 4) separate commits:
>    a) A set of test cases, marked up for the current scheme
>    b) A new Target hook indicating whether unaligned accesses are cheap
>    c) [optional] the CGRecordLowering change just mentioned
>    d) the algorithm change, which updates those tests and makes it easier to see how the behaviour changes.
>    Do you want that commit structure maintained?

That sounds like good commit structure.

On the target hook, it's a shame we can't easily get this information from LLVM.  I believe it's already there — `TargetLowering` has an `allowsMisalignedMemoryAccesses` method that includes some approximation of how fast a particular access would be.  In practice, it seems to be quite complex and often specific to the type and subtarget.  Maybe it'd be worth starting a conversation with LLVM folks to see if this could reasonably be lifted somewhere we could use it?

https://github.com/llvm/llvm-project/pull/65742