Don't merge two constants if we care about the address of both.

This fixes the original testcase in PR8927. It also causes a clang
binary built with a patched clang to increase in size by 0.21%.

We can probably get some of the size back by writing a pass that
detects that a global never has its pointer compared and adds
unnamed_addr to it (maybe extend global opt). It is also possible that
there are some other cases clang could add unnamed_addr to.

I will investigate extending globalopt next.

llvm-svn: 123584

lib/Transforms/IPO/ConstantMerge.cpp

@@ -65,6 +65,18 @@ static void FindUsedValues(GlobalVariable *LLVMUsed,
       UsedValues.insert(GV);
 }
 
+// True if A is better than B.
+static bool IsBetterCannonical(const GlobalVariable &A,
+                               const GlobalVariable &B) {
+  if (!A.hasLocalLinkage() && B.hasLocalLinkage())
+    return true;
+
+  if (A.hasLocalLinkage() && !B.hasLocalLinkage())
+    return false;
+
+  return A.hasUnnamedAddr();
+}
+
 bool ConstantMerge::runOnModule(Module &M) {
   // Find all the globals that are marked "used".  These cannot be merged.
   SmallPtrSet<const GlobalValue*, 8> UsedGlobals;
@@ -85,44 +97,43 @@ bool ConstantMerge::runOnModule(Module &M) {
   // second level constants have initializers which point to the globals that
   // were just merged.
   while (1) {
-    // First pass: identify all globals that can be merged together, filling in
-    // the Replacements vector.  We cannot do the replacement in this pass
-    // because doing so may cause initializers of other globals to be rewritten,
-    // invalidating the Constant* pointers in CMap.
-    //
+
+    // First: Find the canonical constants others will be merged with.
     for (Module::global_iterator GVI = M.global_begin(), E = M.global_end();
          GVI != E; ) {
       GlobalVariable *GV = GVI++;
-      
+
       // If this GV is dead, remove it.
       GV->removeDeadConstantUsers();
       if (GV->use_empty() && GV->hasLocalLinkage()) {
         GV->eraseFromParent();
         continue;
       }
-      
+
       // Only process constants with initializers in the default address space.
       if (!GV->isConstant() || !GV->hasDefinitiveInitializer() ||
           GV->getType()->getAddressSpace() != 0 || GV->hasSection() ||
           // Don't touch values marked with attribute(used).
           UsedGlobals.count(GV))
         continue;
-      
-      // Start by filling slots with only the globals we aren't allowed to
-      // delete because they're externally visible.
-      if (GV->hasLocalLinkage())
-        continue;
-      
+
       Constant *Init = GV->getInitializer();
 
       // Check to see if the initializer is already known.
       GlobalVariable *&Slot = CMap[Init];
 
-      if (Slot == 0) {    // Nope, add it to the map.
+      // If this is the first constant we find or if the old on is local,
+      // replace with the current one. It the current is externally visible
+      // it cannot be replace, but can be the canonical constant we merge with.
+      if (Slot == 0 || IsBetterCannonical(*GV, *Slot)) {
         Slot = GV;
       }
     }
 
+    // Second: identify all globals that can be merged together, filling in
+    // the Replacements vector.  We cannot do the replacement in this pass
+    // because doing so may cause initializers of other globals to be rewritten,
+    // invalidating the Constant* pointers in CMap.
     for (Module::global_iterator GVI = M.global_begin(), E = M.global_end();
          GVI != E; ) {
       GlobalVariable *GV = GVI++;
@@ -134,21 +145,26 @@ bool ConstantMerge::runOnModule(Module &M) {
           UsedGlobals.count(GV))
         continue;
 
-      // Only look at the remaining globals now.
+      // We can only replace constant with local linkage.
       if (!GV->hasLocalLinkage())
         continue;
 
       Constant *Init = GV->getInitializer();
 
       // Check to see if the initializer is already known.
-      GlobalVariable *&Slot = CMap[Init];
+      GlobalVariable *Slot = CMap[Init];
 
-      if (Slot == 0) {    // Nope, add it to the map.
-        Slot = GV;
-      } else {            // Yup, this is a duplicate!
-        // Make all uses of the duplicate constant use the canonical version.
-        Replacements.push_back(std::make_pair(GV, Slot));
-      }
+      if (!Slot || Slot == GV)
+        continue;
+
+      if (!Slot->hasUnnamedAddr() && !GV->hasUnnamedAddr())
+        continue;
+
+      if (!GV->hasUnnamedAddr())
+        Slot->setUnnamedAddr(false);
+
+      // Make all uses of the duplicate constant use the canonical version.
+      Replacements.push_back(std::make_pair(GV, Slot));
     }
 
     if (Replacements.empty())

test/Transforms/ConstantMerge/2011-01-15-EitherOrder.ll

@@ -5,7 +5,7 @@ declare i32 @zed(%struct.foobar*, %struct.foobar*)
 
 %struct.foobar = type { i32 }
 ; CHECK: bar.d
-@bar.d =  constant %struct.foobar zeroinitializer, align 4
+@bar.d =  unnamed_addr constant %struct.foobar zeroinitializer, align 4
 ; CHECK-NOT: foo.d
 @foo.d = internal constant %struct.foobar zeroinitializer, align 4
 define i32 @main() nounwind ssp {

test/Transforms/ConstantMerge/merge-both.ll

@@ -0,0 +1,26 @@
+; RUN: opt -constmerge %s -S -o - | FileCheck %s
+; Test that in one run var3 is merged into var2 and var1 into var4.
+
+declare void @zed(%struct.foobar*, %struct.foobar*)
+
+%struct.foobar = type { i32 }
+
+@var1 = internal constant %struct.foobar { i32 2 }
+@var2 = unnamed_addr constant %struct.foobar { i32 2 }
+@var3 = internal constant %struct.foobar { i32 2 }
+@var4 = unnamed_addr constant %struct.foobar { i32 2 }
+
+; CHECK:      %struct.foobar = type { i32 }
+; CHECK-NOT: @
+; CHECK: @var2 = constant %struct.foobar { i32 2 }
+; CHECK-NEXT: @var4 = constant %struct.foobar { i32 2 }
+; CHECK-NOT: @
+; CHECK: declare void @zed(%struct.foobar*, %struct.foobar*)
+
+define i32 @main() {
+entry:
+  call void @zed(%struct.foobar* @var1, %struct.foobar* @var2)
+  call void @zed(%struct.foobar* @var3, %struct.foobar* @var4)
+  ret i32 0
+}
+

test/Transforms/ConstantMerge/unnamed-addr.ll

@@ -0,0 +1,40 @@
+; RUN: opt -constmerge %s -S -o - | FileCheck %s
+; Test which corresponding x and y are merged and that unnamed_addr
+; is correctly set.
+
+declare void @zed(%struct.foobar*, %struct.foobar*)
+
+%struct.foobar = type { i32 }
+
+@test1.x = internal constant %struct.foobar { i32 1 }
+@test1.y = constant %struct.foobar { i32 1 }
+
+@test2.x = internal constant %struct.foobar { i32 2 }
+@test2.y = unnamed_addr constant %struct.foobar { i32 2 }
+
+@test3.x = internal unnamed_addr constant %struct.foobar { i32 3 }
+@test3.y = constant %struct.foobar { i32 3 }
+
+@test4.x = internal unnamed_addr constant %struct.foobar { i32 4 }
+@test4.y = unnamed_addr constant %struct.foobar { i32 4 }
+
+
+; CHECK:      %struct.foobar = type { i32 }
+; CHECK-NOT: @
+; CHECK: @test1.x = internal constant %struct.foobar { i32 1 }
+; CHECK-NEXT: @test1.y = constant %struct.foobar { i32 1 }
+; CHECK-NEXT: @test2.y = constant %struct.foobar { i32 2 }
+; CHECK-NEXT: @test3.y = constant %struct.foobar { i32 3 }
+; CHECK-NEXT: @test4.y = unnamed_addr constant %struct.foobar { i32 4 }
+; CHECK-NOT: @
+; CHECK: declare void @zed(%struct.foobar*, %struct.foobar*)
+
+define i32 @main() {
+entry:
+  call void @zed(%struct.foobar* @test1.x, %struct.foobar* @test1.y)
+  call void @zed(%struct.foobar* @test2.x, %struct.foobar* @test2.y)
+  call void @zed(%struct.foobar* @test3.x, %struct.foobar* @test3.y)
+  call void @zed(%struct.foobar* @test4.x, %struct.foobar* @test4.y)
+  ret i32 0
+}
+