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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
