Restore previous structure ABI for bitfields with 'packed' attribute for PS4 targets

[ormris]

Summary

An ABI change was introduced in r254596 that modified structure layouts when the 'packed' attribute was used on one-byte bitfields. Since the PS4 target needs to maintain backwards compatibility for all structure layouts, this change reintroduces the old behavior for PS4 targets only. It also introduces PS4 specific cases to the relevant test.

See the following review for discussion of r254596.
https://reviews.llvm.org/D14872

[ormris]

An ABI change was introduced in r254596 that modified structure layouts when the 'packed' attribute was used on one-byte bitfields. Since the PS4 target needs to maintain backwards compatibility for all structure layouts, this change reintroduces the old behavior for PS4 targets only. It also introduces PS4 specific cases to the relevant test.

See the following review for discussion of r254596.
https://reviews.llvm.org/D14872

[probinson]

FTR, from a PS4 perspective this is all good, but we'd like somebody from outside our team to take a look.

[aaron.ballman]

LGTM!

[ormris]

Great. Thanks for the review! I don't have commit access currently.

[aaron.ballman]

I've commit in r310388

include/clang/Basic/DiagnosticSemaKinds.td

 // for their workflow.
 def warn_int_to_pointer_cast : Warning<
   "cast to %1 from smaller integer type %0">,
   InGroup<IntToPointerCast>;
 def warn_int_to_void_pointer_cast : Warning<
   "cast to %1 from smaller integer type %0">,
   InGroup<IntToVoidPointerCast>;
 
+def warn_attribute_ignored_for_field_of_type : Warning<
+  "%0 attribute ignored for field of type %1">,
+  InGroup<IgnoredAttributes>;
 def warn_no_underlying_type_specified_for_enum_bitfield : Warning<
   "enums in the Microsoft ABI are signed integers by default; consider giving "
   "the enum %0 an unsigned underlying type to make this code portable">,
   InGroup<SignedEnumBitfield>, DefaultIgnore;
 def warn_attribute_packed_for_bitfield : Warning<
   "'packed' attribute was ignored on bit-fields with single-byte alignment "
   "in older versions of GCC and Clang">,
   InGroup<DiagGroup<"attribute-packed-for-bitfield">>;

lib/Sema/SemaDeclAttr.cpp

static void handleExtVectorTypeAttr(Sema &S, Scope *scope, Decl *D,
   S.ExtVectorDecls.push_back(cast<TypedefNameDecl>(D));
 }
 
 static void handlePackedAttr(Sema &S, Decl *D, const AttributeList &Attr) {
   if (TagDecl *TD = dyn_cast<TagDecl>(D))
     TD->addAttr(::new (S.Context) PackedAttr(Attr.getRange(), S.Context,
                                         Attr.getAttributeSpellingListIndex()));
   else if (FieldDecl *FD = dyn_cast<FieldDecl>(D)) {
+    bool BitfieldByteAligned = (!FD->getType()->isDependentType() &&
+                                !FD->getType()->isIncompleteType() &&
+                                FD->isBitField() &&
+                                S.Context.getTypeAlign(FD->getType()) <= 8);
+
+    if (S.getASTContext().getTargetInfo().getTriple().isPS4()) {
+      if (BitfieldByteAligned)
+        // The PS4 target needs to maintain ABI backwards compatibility.
+        S.Diag(Attr.getLoc(), diag::warn_attribute_ignored_for_field_of_type)
+          << Attr.getName() << FD->getType();
+      else
+        FD->addAttr(::new (S.Context) PackedAttr(
+                    Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex()));
+    } else {
       // Report warning about changed offset in the newer compiler versions.
-    if (!FD->getType()->isDependentType() &&
-        !FD->getType()->isIncompleteType() && FD->isBitField() &&
-        S.Context.getTypeAlign(FD->getType()) <= 8)
+      if (BitfieldByteAligned)
         S.Diag(Attr.getLoc(), diag::warn_attribute_packed_for_bitfield);
 
       FD->addAttr(::new (S.Context) PackedAttr(
                   Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex()));
+    }
+
   } else
     S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << Attr.getName();
 }
 
 static bool checkIBOutletCommon(Sema &S, Decl *D, const AttributeList &Attr) {
   // The IBOutlet/IBOutletCollection attributes only apply to instance
   // variables or properties of Objective-C classes.  The outlet must also
   // have an object reference type.

test/Sema/struct-packed-align.c

 // RUN: %clang_cc1 %s -fsyntax-only -verify
 // RUN: %clang_cc1 %s -fsyntax-only -triple=x86_64-windows-coff -verify
+// RUN: %clang_cc1 %s -fsyntax-only -triple=x86_64-scei-ps4 -verify
 
 // Packed structs.
 struct s {
     char a;
     int b  __attribute__((packed));
     char c;
     int d;
 };

 
 // Packed attribute shouldn't be ignored for bit-field of char types.
 // Note from GCC reference manual: The 4.1, 4.2 and 4.3 series of GCC ignore
 // the packed attribute on bit-fields of type char. This has been fixed in
 // GCC 4.4 but the change can lead to differences in the structure layout.
 // See the documentation of -Wpacked-bitfield-compat for more information.
 struct packed_chars {
   char a:4;
+#ifdef __ORBIS__
+  // Test for pre-r254596 clang behavior on the PS4 target. PS4 must maintain
+  // ABI backwards compatibility.
+  char b:8 __attribute__ ((packed));
+  // expected-warning@-1 {{'packed' attribute ignored for field of type 'char'}}
+  char c:4;
+#else
   char b:8 __attribute__ ((packed));
   // expected-warning@-1 {{'packed' attribute was ignored on bit-fields with single-byte alignment in older versions of GCC and Clang}}
   char c:4;
+#endif
 };
 
-#if defined(_WIN32) && !defined(__declspec) // _MSC_VER is unavailable in cc1.
+#if (defined(_WIN32) || defined(__ORBIS__)) && !defined(__declspec) // _MSC_VER is unavailable in cc1.
 // On Windows clang uses MSVC compatible layout in this case.
+//
+// Additionally, test for pre-r254596 clang behavior on the PS4 target. PS4
+// must maintain ABI backwards compatibility.
 extern int o1[sizeof(struct packed_chars) == 3 ? 1 : -1];
 extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
 #else
 extern int o1[sizeof(struct packed_chars) == 2 ? 1 : -1];
 extern int o2[__alignof(struct packed_chars) == 1 ? 1 : -1];
 #endif