[cfe-commits] Building EFI with Clang on Mac OS X

[Douglas Gregor]

On Jan 31, 2011, at 3:46 PM, Carl Norum wrote:

> 
> On Jan 31, 2011, at 3:21 PM, Douglas Gregor wrote:
>> Mach-O patch looks good once the LLVM patches are committed.
> 
> Cool - I'll ping here again when those go through on the LLVM side.

I've committed the Mach-O patch as r124660.

>> The MSBitfields bit should be "important", not "benign", since having -mms-bitfields when creating a PCH file but not when using that PCH file would be rather catastrophic. Otherwise, looks good!
> 
> OK - easily done.  I didn't understand what was up there until looking at the warning messages.  Thanks for the heads up.  Updated patch file attached!

I've committed a slightly tweaked version of the MSBitfields patch in r124661. The changes, for your reference, are:

Index: include/clang/Basic/DiagnosticFrontendKinds.td
===================================================================
--- include/clang/Basic/DiagnosticFrontendKinds.td	(revision 124313)
+++ include/clang/Basic/DiagnosticFrontendKinds.td	(working copy)
@@ -150,6 +150,9 @@
 def warn_pch_microsoft_extensions : Error<
     "Microsoft extensions were %select{disabled|enabled}0 in PCH file but are "
     "currently %select{disabled|enabled}1">;
+def warn_pch_ms_bitfields : Error<
+    "Microsoft-compatible structure layout was %select{disabled|enabled}0 in PCH file but is "
+    "currently %select{disabled|enabled}1">;
 def warn_pch_heinous_extensions : Error<
     "heinous extensions were %select{disabled|enabled}0 in PCH file but are "
     "currently %select{disabled|enabled}1">;

Reformatted this to fit in 80 columns.

Index: lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- lib/AST/RecordLayoutBuilder.cpp	(revision 124313)
+++ lib/AST/RecordLayoutBuilder.cpp	(working copy)
@@ -1401,6 +1401,33 @@
     std::pair<uint64_t, unsigned> FieldInfo = Context.getTypeInfo(D->getType());
     FieldSize = FieldInfo.first;
     FieldAlign = FieldInfo.second;
+
+    if (Context.getLangOptions().MSBitfields) {
+      // If MS bitfield layout is required, figure out what type is being
+      // laid out and align the field to the width of that type.
+      
+      // Resolve all typedefs down to their base type and round up the field
+      // alignment if necessary.
+      const BuiltinType *BTy = D->getType()->getAs<BuiltinType>();
+      if (BTy != NULL) {
+        if (FieldSize > FieldAlign) {
+          FieldAlign = FieldSize;
+        }
+      }
+
+      // Check for array type
+      const ArrayType *ATy = Context.getAsArrayType(D->getType());
+      if (ATy != NULL) {
+        QualType ElementType = ATy->getElementType();
+        BTy = ElementType->getAs<BuiltinType>();
+        if (BTy != NULL) {
+          uint64_t ElementSize = Context.getTypeSize(ElementType);
+          if (ElementSize > FieldAlign) {
+            FieldAlign = ElementSize;
+          }
+        }
+      }      
+    }
   }

Simplified this using ASTContext::getBaseElementType(), which also makes sure that we look through all levels of multidimensional arrays to the base element type.

Thanks!

	- Doug