Subzero: Fix emission of global initializers.

Also changes the szbuild.py script to add -fdata-sections, and entirely removes the -disable-globals option.

The global initializer emission basically copies what llc does, based on 3 properties of the global: constant vs non-constant, internal vs external, and full zero-initializer vs non-trivial initializer.

BUG= none
R=jvoung@chromium.org, kschimpf@google.com

Review URL: https://codereview.chromium.org/631383003

diff --git a/pydir/szbuild.py b/pydir/szbuild.py
index 73ac3f2..13eed42 100755 (executable)
--- a/pydir/szbuild.py
+++ b/pydir/szbuild.py
@@ -174,6 +174,7 @@ def ProcessPexe(args, pexe, exe):
             NewerThanOrNotThere(llcbin, obj_llc):
         shellcmd(['pnacl-translate',
                   '-ffunction-sections',
+                  '-fdata-sections',
                   '-c',
                   '-arch', 'x86-32-linux',
                   '-O' + opt_level_map[opt_level],
@@ -193,9 +194,9 @@ def ProcessPexe(args, pexe, exe):
         shellcmd([llvm2ice,
                   '-O' + opt_level,
                   '-bitcode-format=pnacl',
-                  '-disable-globals',
                   '-externalize',
                   '-ffunction-sections',
+                  '-fdata-sections',
                   '-o', asm_sz] +
                  args.sz_args +
                  [pexe],

diff --git a/src/IceClFlags.h b/src/IceClFlags.h
index c434d50..d1a71ae 100644 (file)
--- a/src/IceClFlags.h
+++ b/src/IceClFlags.h
@@ -23,16 +23,14 @@ class ClFlags {
 public:
   ClFlags()
       : DisableInternal(false), SubzeroTimingEnabled(false),
-        DisableTranslation(false), DisableGlobals(false),
-        FunctionSections(false), DataSections(false),
+        DisableTranslation(false), FunctionSections(false), DataSections(false),
         UseIntegratedAssembler(false), UseSandboxing(false), DumpStats(false),
         AllowUninitializedGlobals(false), TimeEachFunction(false),
-        DefaultGlobalPrefix(""), DefaultFunctionPrefix(""),TimingFocusOn(""),
+        DefaultGlobalPrefix(""), DefaultFunctionPrefix(""), TimingFocusOn(""),
         VerboseFocusOn("") {}
   bool DisableInternal;
   bool SubzeroTimingEnabled;
   bool DisableTranslation;
-  bool DisableGlobals;
   bool FunctionSections;
   bool DataSections;
   bool UseIntegratedAssembler;

diff --git a/src/IceConverter.cpp b/src/IceConverter.cpp
index 63ab7af..380f1a0 100644 (file)
--- a/src/IceConverter.cpp
+++ b/src/IceConverter.cpp
@@ -801,8 +801,7 @@ void Converter::convertToIce() {
   TimerMarker T(TimerStack::TT_convertToIce, Ctx);
   nameUnnamedGlobalAddresses(Mod);
   nameUnnamedFunctions(Mod);
-  if (!Ctx->getFlags().DisableGlobals)
-    convertGlobals(Mod);
+  convertGlobals(Mod);
   convertFunctions();
 }
 

diff --git a/src/IceTargetLoweringX8632.cpp b/src/IceTargetLoweringX8632.cpp
index 6829eee..343113c 100644 (file)
--- a/src/IceTargetLoweringX8632.cpp
+++ b/src/IceTargetLoweringX8632.cpp
@@ -4437,70 +4437,52 @@ void TargetGlobalInitX8632::lower(const GlobalAddress &Global,
     return;
 
   Ostream &Str = Ctx->getStrEmit();
-  // constant:
-  //   .section .rodata,"a",@progbits
-  //   .align ALIGN
-  //   .byte ...
-  //   .size NAME, SIZE
-
-  // non-constant:
-  //   .data
-  //   .align ALIGN
-  //   .byte ...
-  //   .size NAME, SIZE
-
-  // zeroinitializer (constant):
-  //   (.section or .data as above)
-  //   .align ALIGN
-  //   .zero SIZE
-  //   .size NAME, SIZE
-
-  // zeroinitializer (non-constant):
-  //   (.section or .data as above)
-  //   .local NAME
-  //   .comm NAME, SIZE, ALIGN
 
   // TODO(kschimpf): Don't mangle name if external and uninitialized. This
   // will allow us to cross test relocations for references to external
   // global variables.
 
-  IceString MangledName = Ctx->mangleName(Global.getName());
-  // Start a new section.
-  if (Ctx->getFlags().DataSections) {
-    Str << "\t.section\t.rodata." << MangledName << ",\"a\",@progbits\n";
-  } else if (Global.getIsConstant()) {
-    Str << "\t.section\t.rodata,\"a\",@progbits\n";
-  } else {
-    Str << "\t.type\t" << MangledName << ",@object\n";
-    Str << "\t.data\n";
-  }
-
-  Str << "\t" << (Global.getIsInternal() ? ".local" : ".global") << "\t"
-      << MangledName << "\n";
-
   const GlobalAddress::InitializerListType &Initializers =
       Global.getInitializers();
-
-  // Note: Handle zero initializations specially when lowering, since
-  // we may be able to reduce object size.
-  GlobalAddress::ZeroInitializer *SimpleZeroInit = nullptr;
-  if (Initializers.size() == 1) {
-    GlobalAddress::Initializer *Init = Initializers[0];
-    if (const auto ZeroInit =
-        llvm::dyn_cast<GlobalAddress::ZeroInitializer>(Init)) {
-      SimpleZeroInit = ZeroInit;
-    }
-  }
-
-  if (SimpleZeroInit && !Global.getIsConstant()) {
-    // TODO(stichnot): Put the appropriate non-constant
-    // zeroinitializers in a .bss section to reduce object size.
-    Str << "\t.comm\t" << MangledName << ", " << Global.getNumBytes() << ", "
-        << Global.getAlignment() << "\n";
-    // }
-  } else {
-    Str << "\t.align\t" << Global.getAlignment() << "\n";
+  assert(Initializers.size());
+  bool HasInitializer =
+      !(Initializers.size() == 1 &&
+        llvm::isa<GlobalAddress::ZeroInitializer>(Initializers[0]));
+  bool IsConstant = Global.getIsConstant();
+  bool IsExternal = !Global.getIsInternal();
+  uint32_t Align = Global.getAlignment();
+  SizeT Size = Global.getNumBytes();
+  IceString MangledName = Ctx->mangleName(Global.getName());
+  IceString SectionSuffix = "";
+  if (Ctx->getFlags().DataSections)
+    SectionSuffix = "." + MangledName;
+
+  Str << "\t.type\t" << MangledName << ",@object\n";
+
+  if (IsConstant)
+    Str << "\t.section\t.rodata" << SectionSuffix << ",\"a\",@progbits\n";
+  else if (HasInitializer)
+    Str << "\t.section\t.data" << SectionSuffix << ",\"aw\",@progbits\n";
+  else if (IsExternal)
+    Str << "\t.section\t.bss" << SectionSuffix << ",\"aw\",@nobits\n";
+  // No .section for non-constant + zeroinitializer + internal
+
+  if (IsExternal)
+    Str << "\t.globl\t" << MangledName << "\n";
+  else if (!IsConstant && !HasInitializer)
+    Str << "\t.local\t" << MangledName << "\n";
+  // Internal symbols only get .local when using .comm.
+
+  if ((IsConstant || HasInitializer || IsExternal) && Align > 1)
+    Str << "\t.align\t" << Align << "\n";
+  // Alignment is part of .comm.
+
+  if (IsConstant || HasInitializer || IsExternal)
     Str << MangledName << ":\n";
+  else
+    Str << "\t.comm\t" << MangledName << "," << Size << "," << Align << "\n";
+
+  if (HasInitializer) {
     for (GlobalAddress::Initializer *Init : Initializers) {
       switch (Init->getKind()) {
       case GlobalAddress::Initializer::DataInitializerKind: {
@@ -4539,8 +4521,13 @@ void TargetGlobalInitX8632::lower(const GlobalAddress &Global,
       }
       }
     }
-    Str << "\t.size\t" << MangledName << ", " << Global.getNumBytes() << "\n";
-  }
+  } else if (IsConstant || IsExternal)
+    Str << "\t.zero\t" << Size << "\n";
+  // Size is part of .comm.
+
+  if (IsConstant || HasInitializer || IsExternal)
+    Str << "\t.size\t" << MangledName << ", " << Size << "\n";
+  // Size is part of .comm.
 }
 
 } // end of namespace Ice

diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index 15272c1..6642ba9 100644 (file)
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -2369,8 +2369,7 @@ private:
         }
       }
       Trans.nameUnnamedFunctions(Context->getModule());
-      if (!getFlags().DisableGlobals)
-        getTranslator().lowerGlobals(Context->getGlobalIDAddresses());
+      getTranslator().lowerGlobals(Context->getGlobalIDAddresses());
       GlobalAddressNamesAndInitializersInstalled = true;
     }
   }

diff --git a/src/llvm2ice.cpp b/src/llvm2ice.cpp
index 637eb16..3546b84 100644 (file)
--- a/src/llvm2ice.cpp
+++ b/src/llvm2ice.cpp
@@ -111,10 +111,6 @@ static cl::opt<std::string> VerboseFocusOn(
     cl::desc("Temporarily enable full verbosity for a specific function"),
     cl::init(""));
 
-static cl::opt<bool>
-DisableGlobals("disable-globals",
-               cl::desc("Disable global initializer translation"));
-
 // This is currently unused, and is a placeholder for lit tests.
 static cl::opt<bool>
     DisablePhiEdgeSplit("no-phi-edge-split",
@@ -197,7 +193,6 @@ int main(int argc, char **argv) {
   Flags.DisableInternal = DisableInternal;
   Flags.SubzeroTimingEnabled = SubzeroTimingEnabled;
   Flags.DisableTranslation = DisableTranslation;
-  Flags.DisableGlobals = DisableGlobals;
   Flags.FunctionSections = FunctionSections;
   Flags.DataSections = DataSections;
   Flags.UseIntegratedAssembler = UseIntegratedAssembler;

diff --git a/tests_lit/llvm2ice_tests/globalinit.pnacl.ll b/tests_lit/llvm2ice_tests/globalinit.pnacl.ll
index be6f4c9..ba612e3 100644 (file)
--- a/tests_lit/llvm2ice_tests/globalinit.pnacl.ll
+++ b/tests_lit/llvm2ice_tests/globalinit.pnacl.ll
@@ -6,63 +6,63 @@
 ; RUN: %p2i -i %s --args --verbose none | FileCheck --check-prefix=ERRORS %s
 
 @PrimitiveInit = internal global [4 x i8] c"\1B\00\00\00", align 4
-; CHECK: .data
-; CHECK-NEXT: .local
+; CHECK: .type PrimitiveInit,@object
+; CHECK-NEXT: .section .data,"aw",@progbits
 ; CHECK-NEXT: .align 4
 ; CHECK-NEXT: PrimitiveInit:
 ; CHECK-NEXT: .byte
 ; CHECK: .size PrimitiveInit, 4
 
 @PrimitiveInitConst = internal constant [4 x i8] c"\0D\00\00\00", align 4
-; CHECK: .section .rodata,"a",@progbits
-; CHECK-NEXT: .local
+; CHECK: .type PrimitiveInitConst,@object
+; CHECK-NEXT: .section .rodata,"a",@progbits
 ; CHECK-NEXT: .align 4
 ; CHECK-NEXT: PrimitiveInitConst:
 ; CHECK-NEXT: .byte
 ; CHECK: .size PrimitiveInitConst, 4
 
 @ArrayInit = internal global [20 x i8] c"\0A\00\00\00\14\00\00\00\1E\00\00\00(\00\00\002\00\00\00", align 4
-; CHECK: .data
-; CHECK-NEXT: .local
+; CHECK: .type ArrayInit,@object
+; CHECK-NEXT: .section .data,"aw",@progbits
 ; CHECK-NEXT: .align 4
 ; CHECK-NEXT: ArrayInit:
 ; CHECK-NEXT: .byte
 ; CHECK: .size ArrayInit, 20
 
 @ArrayInitPartial = internal global [40 x i8] c"<\00\00\00F\00\00\00P\00\00\00Z\00\00\00d\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00", align 4
-; CHECK: .data
-; CHECK-NEXT: .local
+; CHECK: .type ArrayInitPartial,@object
+; CHECK-NEXT: .section .data,"aw",@progbits
 ; CHECK-NEXT: .align 4
 ; CHECK-NEXT: ArrayInitPartial:
 ; CHECK-NEXT: .byte
 ; CHECK: .size ArrayInitPartial, 40
 
 @PrimitiveInitStatic = internal global [4 x i8] zeroinitializer, align 4
-; CHECK: .data
+; CHECK: .type PrimitiveInitStatic,@object
 ; CHECK-NEXT: .local PrimitiveInitStatic
-; CHECK-NEXT: .comm PrimitiveInitStatic, 4, 4
+; CHECK-NEXT: .comm PrimitiveInitStatic,4,4
 
 @PrimitiveUninit = internal global [4 x i8] zeroinitializer, align 4
-; CHECK: .data
+; CHECK: .type PrimitiveUninit,@object
 ; CHECK-NEXT: .local PrimitiveUninit
-; CHECK-NEXT: .comm PrimitiveUninit, 4, 4
+; CHECK-NEXT: .comm PrimitiveUninit,4,4
 
 @ArrayUninit = internal global [20 x i8] zeroinitializer, align 4
-; CHECK: .data
+; CHECK: .type ArrayUninit,@object
 ; CHECK-NEXT: .local ArrayUninit
-; CHECK-NEXT: .comm ArrayUninit, 20, 4
+; CHECK-NEXT: .comm ArrayUninit,20,4
 
 @ArrayUninitConstDouble = internal constant [200 x i8] zeroinitializer, align 8
-; CHECK: .section .rodata,"a",@progbits
-; CHECK-NEXT: .local
+; CHECK: .type ArrayUninitConstDouble,@object
+; CHECK-NEXT: .section .rodata,"a",@progbits
 ; CHECK-NEXT: .align 8
 ; CHECK-NEXT: ArrayUninitConstDouble:
 ; CHECK-NEXT: .zero 200
 ; CHECK-NEXT: .size ArrayUninitConstDouble, 200
 
 @ArrayUninitConstInt = internal constant [20 x i8] zeroinitializer, align 4
+; CHECK: .type ArrayUninitConstInt,@object
 ; CHECK: .section .rodata,"a",@progbits
-; CHECK-NEXT: .local
 ; CHECK-NEXT: .align 4
 ; CHECK-NEXT: ArrayUninitConstInt:
 ; CHECK-NEXT: .zero 20

diff --git a/tests_lit/reader_tests/globalinit.pnacl.ll b/tests_lit/reader_tests/globalinit.pnacl.ll
index 4bdb4f8..cd4fda3 100644 (file)
--- a/tests_lit/reader_tests/globalinit.pnacl.ll
+++ b/tests_lit/reader_tests/globalinit.pnacl.ll
@@ -11,63 +11,63 @@
 ; RUN: %p2i -i %s --args -verbose none | FileCheck --check-prefix=ERRORS %s
 
 @PrimitiveInit = internal global [4 x i8] c"\1B\00\00\00", align 4
-; CHECK: .data
-; CHECK-NEXT: .local
+; CHECK: .type PrimitiveInit,@object
+; CHECK-NEXT: .section .data,"aw",@progbits
 ; CHECK-NEXT: .align 4
 ; CHECK-NEXT: PrimitiveInit:
 ; CHECK-NEXT: .byte
 ; CHECK: .size PrimitiveInit, 4
 
 @PrimitiveInitConst = internal constant [4 x i8] c"\0D\00\00\00", align 4
-; CHECK: .section .rodata,"a",@progbits
-; CHECK-NEXT: .local
+; CHECK: .type PrimitiveInitConst,@object
+; CHECK-NEXT: .section .rodata,"a",@progbits
 ; CHECK-NEXT: .align 4
 ; CHECK-NEXT: PrimitiveInitConst:
 ; CHECK-NEXT: .byte
 ; CHECK: .size PrimitiveInitConst, 4
 
 @ArrayInit = internal global [20 x i8] c"\0A\00\00\00\14\00\00\00\1E\00\00\00(\00\00\002\00\00\00", align 4
-; CHECK: .data
-; CHECK-NEXT: .local
+; CHECK: .type ArrayInit,@object
+; CHECK-NEXT: .section .data,"aw",@progbits
 ; CHECK-NEXT: .align 4
 ; CHECK-NEXT: ArrayInit:
 ; CHECK-NEXT: .byte
 ; CHECK: .size ArrayInit, 20
 
 @ArrayInitPartial = internal global [40 x i8] c"<\00\00\00F\00\00\00P\00\00\00Z\00\00\00d\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00\00", align 4
-; CHECK: .data
-; CHECK-NEXT: .local
+; CHECK: .type ArrayInitPartial,@object
+; CHECK-NEXT: .section .data,"aw",@progbits
 ; CHECK-NEXT: .align 4
 ; CHECK-NEXT: ArrayInitPartial:
 ; CHECK-NEXT: .byte
 ; CHECK: .size ArrayInitPartial, 40
 
 @PrimitiveInitStatic = internal global [4 x i8] zeroinitializer, align 4
-; CHECK: .data
+; CHECK: .type PrimitiveInitStatic,@object
 ; CHECK-NEXT: .local PrimitiveInitStatic
-; CHECK-NEXT: .comm PrimitiveInitStatic, 4, 4
+; CHECK-NEXT: .comm PrimitiveInitStatic,4,4
 
 @PrimitiveUninit = internal global [4 x i8] zeroinitializer, align 4
-; CHECK: .data
+; CHECK: .type PrimitiveUninit,@object
 ; CHECK-NEXT: .local PrimitiveUninit
-; CHECK-NEXT: .comm PrimitiveUninit, 4, 4
+; CHECK-NEXT: .comm PrimitiveUninit,4,4
 
 @ArrayUninit = internal global [20 x i8] zeroinitializer, align 4
-; CHECK: .data
+; CHECK: .type ArrayUninit,@object
 ; CHECK-NEXT: .local ArrayUninit
-; CHECK-NEXT: .comm ArrayUninit, 20, 4
+; CHECK-NEXT: .comm ArrayUninit,20,4
 
 @ArrayUninitConstDouble = internal constant [200 x i8] zeroinitializer, align 8
-; CHECK: .section .rodata,"a",@progbits
-; CHECK-NEXT: .local
+; CHECK: .type ArrayUninitConstDouble,@object
+; CHECK-NEXT: .section .rodata,"a",@progbits
 ; CHECK-NEXT: .align 8
 ; CHECK-NEXT: ArrayUninitConstDouble:
 ; CHECK-NEXT: .zero 200
 ; CHECK-NEXT: .size ArrayUninitConstDouble, 200
 
 @ArrayUninitConstInt = internal constant [20 x i8] zeroinitializer, align 4
+; CHECK: .type ArrayUninitConstInt,@object
 ; CHECK: .section .rodata,"a",@progbits
-; CHECK-NEXT: .local
 ; CHECK-NEXT: .align 4
 ; CHECK-NEXT: ArrayUninitConstInt:
 ; CHECK-NEXT: .zero 20