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

[Douglas Gregor]

Hi Carl,

On Jan 27, 2011, at 12:02 PM, Carl Norum wrote:

> 
> Hi folks,
> 
> I've attached a patch for review; these are the changes we use to build EFI for Mac boot ROMs using clang/llvm on Mac OS X.  The simplest explanation is that it's a partial implementation of gcc's "-mms-bitfields" flag for clang.
> 
> Let me know what you think!

A couple of comments...

Index: test/Misc/mms-bitfields.c
===================================================================
--- test/Misc/mms-bitfields.c	(revision 0)
+++ test/Misc/mms-bitfields.c	(revision 0)
@@ -0,0 +1,9 @@
+// RUN: %clang -arch i386 -mms-bitfields -emit-llvm -c %s -o %t
+// RUN: llvm-dis %t -o - | FileCheck %s
+
+struct s {
+  int       f32;
+  long long f64;
+} s;
+
+// CHECK: @s = common global %struct.s zeroinitializer, align 8

Instead of using %clang and llvm-dis in the RUN line, it'd be far better to use 

	%clang_cc1 -triple <triple that makes sense for -mms-bitfields> -emit-llvm -o - 

and pipe the results to FileCheck. The important part is the use of %clang_cc1 with -triple, so the test is completely independent of the host environment.

This test seems like it's not really testing bit-field layout at all...

Index: include/clang/Basic/LangOptions.h
===================================================================
--- include/clang/Basic/LangOptions.h	(revision 124313)
+++ include/clang/Basic/LangOptions.h	(working copy)
@@ -56,6 +56,7 @@
   unsigned SjLjExceptions    : 1;  // Use setjmp-longjump exception handling.
   unsigned RTTI              : 1;  // Support RTTI information.
 
+  unsigned MSBitfields       : 1; // MS-compatible structure layout
   unsigned NeXTRuntime       : 1; // Use NeXT runtime.
   unsigned Freestanding      : 1; // Freestanding implementation
   unsigned NoBuiltin         : 1; // Do not use builtin functions (-fno-builtin)
@@ -163,6 +164,7 @@
     C99 = Microsoft = Borland = CPlusPlus = CPlusPlus0x = 0;
     CXXOperatorNames = PascalStrings = WritableStrings = ConstStrings = 0;
     Exceptions = SjLjExceptions = Freestanding = NoBuiltin = 0;
+    MSBitfields = 0;
     NeXTRuntime = 1;
     RTTI = 1;
     LaxVectorConversions = 1;

Please update the AST reader and writer, so that the MSBitfields bit is properly saved/loaded/checked.

Index: lib/Basic/Targets.cpp
===================================================================
--- lib/Basic/Targets.cpp	(revision 124313)
+++ lib/Basic/Targets.cpp	(working copy)
@@ -2675,7 +2675,10 @@
     case llvm::Triple::MinGW64:
       return new MinGWX86_64TargetInfo(T);
     case llvm::Triple::Win32:   // This is what Triple.h supports now.
-      return new VisualStudioWindowsX86_64TargetInfo(T);
+      if (Triple.getEnvironment() == llvm::Triple::MachO)
+        return new DarwinX86_64TargetInfo(T);
+      else
+        return new VisualStudioWindowsX86_64TargetInfo(T);
     default:
       return new X86_64TargetInfo(T);
     }

This change should be committed separately.

Index: lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- lib/AST/RecordLayoutBuilder.cpp	(revision 124313)
+++ lib/AST/RecordLayoutBuilder.cpp	(working copy)
@@ -1401,6 +1401,25 @@
     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.
+      const Type *T = D->getType().getTypePtr();
+      Type::TypeClass TC = T->getTypeClass();
+
+      // Resolve all typedefs down to their base type
+      while (TC == Type::Typedef) {
+        const TypedefDecl *Typedef = cast<TypedefType>(T)->getDecl();
+        T = Typedef->getUnderlyingType().getTypePtr();
+        TC = T->getTypeClass();
+      }
+
+      // Round up the field alignment if necessary.
+      if ((TC == Type::Builtin) && (FieldSize > FieldAlign)) {
+        FieldAlign = FieldSize;
+      }
+    }
   }

It's best not to try to walk through typedefs manually; Type's getAs member function template will do that for you. You can collapse most of that logic down to

	if (const BuiltinType *BT = D->getType()->getAs<BuiltinType>())
	  if (FieldSize > FieldAlign)
		FieldAlign = FieldSize;

However, I think you also want to look through array types, first? ASTContext has a routine to do that.

Thanks for working on this! Do we have a good sense of what else is needed for -mms-bitfields? Documentation on this feature seems a bit scarce.

	- Doug
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110131/93b22e43/attachment.html>