Place undefined globals in .bss instead of .data

[varkor]

Summary

Following up on the discussion from http://lists.llvm.org/pipermail/llvm-dev/2017-April/112305.html, undef values are now placed in the .bss as well as null values. This prevents undef global values taking up potentially huge amounts of space in the .data section.

The following two lines now both generate equivalent .bss data:

@vals1 = internal unnamed_addr global [20000000 x i32] zeroinitializer, align 4
@vals2 = internal unnamed_addr global [20000000 x i32] undef, align 4 ; previously unaccounted for

This is primarily motivated by the corresponding issue in the Rust compiler.

[efriedma]

Please post diffs with context (see https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface).

Needs a regression test (probably in test/CodeGen/X86), to show that we are in fact generating the expected code.

Does this do the right thing for constant structs which have some null and some undef members?

[varkor]

• Added support for structs with a combination of null and undef fields.
• Added a regression test.

[efriedma]

lib/Target/TargetLoweringObjectFile.cpp

80

What if E is a ConstantAggregate?

[varkor]

Added support for nested constant aggregates.

[varkor]

@efriedma: are there any other changes you can see that need to be made here?

[efriedma]

lib/Target/TargetLoweringObjectFile.cpp

63

Maybe "for (auto Operand : C->operand_values())"?

[varkor]

Use range-based for loop.

[efriedma]

LGTM. Do you have commit access?

[varkor]

Done. That is slightly neater!

[varkor]

No, I don't have commit access — if you could do that for me, that'd be great! Thank you!

[efriedma]

I was going to apply this, but then I found there's a "make check" failure caused by this change:

[...]/llvm/test/CodeGen/ARM/memfunc.ll:425:10: error: expected string not found in input
; CHECK: arr8:
         ^
<stdin>:341:1: note: scanning from here
_arr9:
^
<stdin>:341:2: note: possible intended match here
_arr9:
 ^

Probably the test just needs to be updated, but please take a look.

[varkor]

Fixed an ARM CodeGen test whose behaviour was changed by the patch. (The data for arr8 is now placed in the BSS, so the simplest fix seemed just to be no longer checking for p2align, which is no longer present.)

[varkor]

As Rafael pointed out, we should still be checking the alignment of bss data — it's just no longer specified with p2align. I've updated the test to check for the alignment of arr8. I think this should now be ready to take a look at again!

[efriedma]

LGTM.

How should I credit you in the commit message?

[varkor]

Great, thanks! Just as "varkor" is great.

lib/Target/TargetLoweringObjectFile.cpp

void TargetLoweringObjectFile::Initialize(MCContext &ctx,
   InitMCObjectFileInfo(TM.getTargetTriple(), TM.isPositionIndependent(),
                        TM.getCodeModel(), *Ctx);
 }
 
 TargetLoweringObjectFile::~TargetLoweringObjectFile() {
   delete Mang;
 }
 
+static bool isNullOrUndef(const Constant *C) {
+  // Check that the constant isn't all zeros or undefs.
+  if (C->isNullValue() || isa<UndefValue>(C))
+    return true;
+  if (!isa<ConstantAggregate>(C))
+    return false;
+  for (auto Operand : C->operand_values()) {
+    if (!isNullOrUndef(cast<Constant>(Operand)))
+      return false;
+  }
+  return true;
+}
+
 static bool isSuitableForBSS(const GlobalVariable *GV, bool NoZerosInBSS) {
   const Constant *C = GV->getInitializer();
 
   // Must have zero initializer.
-  if (!C->isNullValue())
+  if (!isNullOrUndef(C))
     return false;
 
   // Leave constant zeros in readonly constant sections, so they can be shared.
   if (GV->isConstant())
     return false;
 
   // If the global has an explicit section specified, don't put it in BSS.
   if (GV->hasSection())
     return false;
 
   // If -nozero-initialized-in-bss is specified, don't ever use BSS.
   if (NoZerosInBSS)
     return false;
 

test/CodeGen/ARM/memfunc.ll

 ; CHECK: arr3:
 ; CHECK-NOT: .p2align
 ; CHECK: arr4:
 ; CHECK: {{\.data|\.section.+data}}
 ; CHECK-NOT: .p2align
 ; CHECK: arr5:
 ; CHECK-NOT: .p2align
 ; CHECK: arr6:
-; CHECK: .p2align 4
-; CHECK: arr8:
+; CHECK-NOT: .p2align
+; CHECK: arr8
 ; CHECK: .p2align 4
 ; CHECK: arr9:
 
 ; CHECK-NOT: arr7:
 
 declare void @llvm.memmove.p0i8.p0i8.i32(i8* nocapture, i8* nocapture, i32, i32, i1) nounwind
 declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture, i32, i32, i1) nounwind
 declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i32, i1) nounwind

test/CodeGen/X86/undef-globals-bss.ll

+; RUN: llc < %s | FileCheck %s
+target triple = "x86_64-apple-darwin"
+
+; CHECK: .zerofill __DATA,__bss,_vals,8000000,2 ## @vals
+@vals = internal unnamed_addr global [2000000 x i32] undef, align 4
+
+; CHECK: .zerofill __DATA,__bss,_struct,8000040,3 ## @struct
+@struct = internal global { i1, [8000000 x i8], [7 x i8], i64, { [4 x i32], { i8 }, i1 } }
+                { i1 false, [8000000 x i8] zeroinitializer, [7 x i8] undef, i64 0,
+                        { [4 x i32], { i8 }, i1 }
+                        { [4 x i32] zeroinitializer, { i8 } { i8 undef }, i1 false }
+                }, align 8