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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to