[PATCH] D14980: PR18513: make gcc compatible layout for bit-fields with explicit aligned attribute

[Dmitry Polukhin via cfe-commits]

DmitryPolukhin added a comment.

This CL doesn't changes anything for ms_struct cases and ms_struct seems to be broken for bit-fields even for very simple cases so filed separate bug https://llvm.org/bugs/show_bug.cgi?id=25707

PTAL


================
Comment at: lib/AST/RecordLayoutBuilder.cpp:1606
@@ -1605,1 +1605,3 @@
+    } else if (ExplicitFieldAlign)
+      FieldOffset = llvm::RoundUpToAlignment(FieldOffset, ExplicitFieldAlign);
 
----------------
rjmccall wrote:
> It does still seem to be limited by platforms that ignore natural alignment, though, and by #pragma pack (2) and so on, so at least some of the modifications made to FieldAlign need to be reflected in ExplicitFieldAlign.
> 
> I had to spend some time trying to work out counter-examples, but I did convince myself that the effect of the fallback here is correct.  My concern was that there was a way we might round up to FieldAlign when we only needed to round up to ExplicitFieldAlign, which could matter if there were another properly-aligned position within the current storage unit.  However, the second clause of the first condition will only trigger at the very end of a storage unit, which means that there cannot be any other properly-aligned positions within it.
> 
> It might be wrong for zero-width bit-fields, though, since we'll always round up to FieldAlign instead of ExplicitFieldAlign.
I think compiler can ignore natural type alignment here because it is bit-field and therefore compiler has to generate shift/mask sequence for accessing unaligned field anyway. I tried examples like:

struct __attribute__((packed)) B {
  char AField;
  __attribute__((aligned(1))) __int128 i : 124;
  char BField;
};

Both GCC and Clang with my patch generates the same result on x86 and ARM:
sizeof(struct B) = 18
offsetof(struct B, BField) = 17
__alignof(struct B) = 1

Also tried zero-width bit-filed, my patch works fine. So I was not able to find arch that requires round up for ExplicitFieldAlign and what is required. All examples that I can think of work identical on GCC and Clang.


http://reviews.llvm.org/D14980