[PATCH] D16586: Make clang AAPCS compliant w.r.t volatile bitfield accesses

[Asiri Rathnayake via cfe-commits]

rmaprath created this revision.
rmaprath added reviewers: rjmccall, jmolloy, rengolin, olista01.
rmaprath added a subscriber: cfe-commits.
Herald added a subscriber: aemerson.

Lets consider the following plain struct:
```
  struct S1 {
    char a;
    short b : 8;
  };
```
According to the AAPCS, the bitfield 'b' _can_ be accessed by loading its
"container" (a short in this case) and then using bit-shifting operations to
extract the actual bitfield value. However, for plain (non-volatile) bitfields,
the AAPCS does not mandate this, so compilers are free to use whatever type of
access they think is best. armcc tends to use wider accesses even when narrower
accesses are possible (like in the above example, b can be byte-loaded), whereas
clang and gcc tend to issue narrow loads/stores where available.

But things get tricky when volatile bitfields are involved:
```
  struct S2 {
    char a;
    volatile short b : 8;
  };
```
Now the AAPCS mandates that 'b' must be accessed through loads/stores of widths
as appropriate to the container type (Section 7.1.7.5). To complicate matters
further, AAPCS allows overlapping of bitfields with regular fields
(Section 7.1.7.4). What this means is that the storage units of 'a' and 'b'
are overlapping, where 'a' occupies the first byte of the struct and 'b' lies
on the first half-word of the struct. Loading 'b' will load 'a' as well since
'b' is loaded as a short.

Currently, clang will byte-load 'b' and is therefore not AAPCS compliant. The
purpose of this patch is to make clang respect these access widths of volatile
bitfields.

One way to fix this problem is to teach clang the concept of overlapping storage
units, this would require an overhaul of clang's structure layout code, as the
idea of distinct storage units is deeply embedded in clang's structure layout
routines. In this patch, I take the view that this confusion is only a matter of
abstraction; whether the above struct's storage is viewed as two consecutive
bytes (this is how clang lays out the struct) or as a single half-word (this is
how armcc lays out the struct) is irrelevant, the actual data of the fields will
always be at the same offset from the base of the struct. This difference of
abstraction only matters when we generate code to access the relevant fields, we
can therefore intercept those loads/stores in clang and adjust the accesses so
that they are AAPCS compliant.

http://reviews.llvm.org/D16586

Files:
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGRecordLayoutBuilder.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGen/aapcs-bitfield.c
  test/CodeGen/arm-bitfield-alignment.c

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D16586.46003.patch
Type: text/x-patch
Size: 27318 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160126/b5735a08/attachment-0001.bin>