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

Nathan Sidwell via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 18 10:07:31 PDT 2024


urnathan wrote:

@rjmccall here is a rebase an update, which I think addresses all your comments.  I did make some material changes though:

1) I removed the Volatile handling. I was always a little uncomfortable with it because it didn't affect the access units of a non-volatile bitfield that ends up being volatile qualified via the structure's volatile quals itself. Users of volatile fields are probably confused, but if not they have a std-blessed mechanism for isolating access units -- a zero-length bitfield. The heuristic broke the idea that this patch /just/ implemented a better access algorithm so I was being inconsistent. AFAICT the ARM ABI, which seems to be one that describes volatile bitfields carefully, does not specify that volatile bitfields should  be as isolated as possible.  This change causes one change, in `CodeGen/aapcs-bitfield.c` with:
```
struct st5 {
  int a : 12;
  volatile char c : 5;
} st5;
```
The two bitfields are placed into a single 32-bit access unit, rather than separate 16bit ones. Either behaviour is ok with the aapcs. (The previous algorithm would have placed them into a single 24bit field if they abutted at a byte boundary, no change in that behaviour now.)

2) Implementing the barrier behaviour you wanted. I tried to find a psABI that had the right attributes to place barriers at arbitrary bit positions, to see if it had any comment, but failed to find one.  as you say, this simplifies things, but ...

3) The barrier bitfield behaviour that we already had showed an interesting behaviour, which I suspect is from a later scissoring processing.  Namely with:
```
struct  A {
  unsigned a : 16;
  unsigned b : 8;
  char : 0;
  unsigned d;
} a;
```
we'd generate an llvm struct of `{ i24, i32}`, but then adjust the access unit for the bitfields to be a 32-bit unit itself. That seems conformant, because nothing else uses that 4th byte, and the std merely says that the bitfield starts at an allocation unit boundary. This patch was originally treating that barrier as a limit to the current span, whereas we can use the next member with storage as that limit.  This is actually the behaviour we had when we reached the end of a run of bitfields, so I have made that change as you can see from the changed handling setting Barrier.

4) I adjusted the new testcases to reduce their complexity -- we don't care about endianness, which only affects the placement of the bitfields within their access unit, not the access units themselves.

It may be that the later pass that was adjusting bitfield accesses to natural sizes can be simplified or deleted. I've not investigated.

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


More information about the cfe-commits mailing list