[llvm] r281538 - [LTO] Fix commons handling

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 14 14:05:04 PDT 2016


Author: mehdi_amini
Date: Wed Sep 14 16:05:04 2016
New Revision: 281538

URL: http://llvm.org/viewvc/llvm-project?rev=281538&view=rev
Log:
[LTO] Fix commons handling

Previously the prevailing information was not honored, and commons
symbols could override a strong definition. This patch fixes it and
propose the following semantic for commons: the client should mark
as prevailing the commons that it expects the LTO implementation to
merge (i.e. take the maximum size and alignment).
It implies that commons are allowed to have multiple prevailing
definitions.

Differential Revision: https://reviews.llvm.org/D24545

Added:
    llvm/trunk/test/LTO/Resolution/X86/Inputs/commons.ll
    llvm/trunk/test/LTO/Resolution/X86/commons.ll
Modified:
    llvm/trunk/include/llvm/LTO/LTO.h
    llvm/trunk/lib/LTO/LTO.cpp
    llvm/trunk/test/tools/llvm-lto2/X86/common.ll

Modified: llvm/trunk/include/llvm/LTO/LTO.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/LTO/LTO.h?rev=281538&r1=281537&r2=281538&view=diff
==============================================================================
--- llvm/trunk/include/llvm/LTO/LTO.h (original)
+++ llvm/trunk/include/llvm/LTO/LTO.h Wed Sep 14 16:05:04 2016
@@ -315,6 +315,8 @@ private:
     struct CommonResolution {
       uint64_t Size = 0;
       unsigned Align = 0;
+      /// Record if at least one instance of the common was marked as prevailing
+      bool Prevailing = false;
     };
     std::map<std::string, CommonResolution> Commons;
 

Modified: llvm/trunk/lib/LTO/LTO.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTO.cpp?rev=281538&r1=281537&r2=281538&view=diff
==============================================================================
--- llvm/trunk/lib/LTO/LTO.cpp (original)
+++ llvm/trunk/lib/LTO/LTO.cpp Wed Sep 14 16:05:04 2016
@@ -352,13 +352,14 @@ Error LTO::addRegularLTO(std::unique_ptr
         break;
       }
     }
-    // Common resolution: collect the maximum size/alignment.
-    // FIXME: right now we ignore the prevailing information, it is not clear
-    // what is the "right" behavior here.
+    // Common resolution: collect the maximum size/alignment over all commons.
+    // We also record if we see an instance of a common as prevailing, so that
+    // if none is prevailing we can ignore it later.
     if (Sym.getFlags() & object::BasicSymbolRef::SF_Common) {
       auto &CommonRes = RegularLTO.Commons[Sym.getIRName()];
       CommonRes.Size = std::max(CommonRes.Size, Sym.getCommonSize());
       CommonRes.Align = std::max(CommonRes.Align, Sym.getCommonAlignment());
+      CommonRes.Prevailing |= Res.Prevailing;
     }
 
     // FIXME: use proposed local attribute for FinalDefinitionInLinkageUnit.
@@ -421,6 +422,9 @@ Error LTO::runRegularLTO(AddOutputFn Add
   // all the prevailing when adding the inputs, and we apply it here.
   const DataLayout &DL = RegularLTO.CombinedModule->getDataLayout();
   for (auto &I : RegularLTO.Commons) {
+    if (!I.second.Prevailing)
+      // Don't do anything if no instance of this common was prevailing.
+      continue;
     GlobalVariable *OldGV = RegularLTO.CombinedModule->getNamedGlobal(I.first);
     if (OldGV && DL.getTypeAllocSize(OldGV->getValueType()) == I.second.Size) {
       // Don't create a new global if the type is already correct, just make

Added: llvm/trunk/test/LTO/Resolution/X86/Inputs/commons.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/Resolution/X86/Inputs/commons.ll?rev=281538&view=auto
==============================================================================
--- llvm/trunk/test/LTO/Resolution/X86/Inputs/commons.ll (added)
+++ llvm/trunk/test/LTO/Resolution/X86/Inputs/commons.ll Wed Sep 14 16:05:04 2016
@@ -0,0 +1,4 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at x = global i32 42, align 4

Added: llvm/trunk/test/LTO/Resolution/X86/commons.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/Resolution/X86/commons.ll?rev=281538&view=auto
==============================================================================
--- llvm/trunk/test/LTO/Resolution/X86/commons.ll (added)
+++ llvm/trunk/test/LTO/Resolution/X86/commons.ll Wed Sep 14 16:05:04 2016
@@ -0,0 +1,12 @@
+; RUN: llvm-as -o %t1.bc %s
+; RUN: llvm-as -o %t2.bc %p/Inputs/commons.ll
+; RUN: llvm-lto2 %t1.bc -r=%t1.bc,x,l %t2.bc -r=%t2.bc,x,pl -o %t.out -save-temps
+; RUN: llvm-dis -o - %t.out.0.0.preopt.bc  | FileCheck %s
+
+; A strong definition should override the common
+; CHECK: @x = global i32 42, align 4
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at x = common global i16 0, align 2

Modified: llvm/trunk/test/tools/llvm-lto2/X86/common.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-lto2/X86/common.ll?rev=281538&r1=281537&r2=281538&view=diff
==============================================================================
--- llvm/trunk/test/tools/llvm-lto2/X86/common.ll (original)
+++ llvm/trunk/test/tools/llvm-lto2/X86/common.ll Wed Sep 14 16:05:04 2016
@@ -19,7 +19,6 @@
 ; RUN:  -r %t2.bc,bar,px
 ; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck %s --check-prefix=LARGE-PREVAILED
 
-
 ; Client marked the "small with large alignment" one as prevailing
 ; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \
 ; RUN:  -r %t1.bc,v,px \
@@ -53,16 +52,37 @@
 ; RUN:  -r %t2.bc,bar,px
 ; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck  %s --check-prefix=NONE-PREVAILED2
 
+
+
+; Client marked both as prevailing
+; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN:  -r %t1.bc,v,px \
+; RUN:  -r %t2.bc,v,px \
+; RUN:  -r %t1.bc,foo,px \
+; RUN:  -r %t2.bc,bar,px
+; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck %s --check-prefix=BOTH-PREVAILED1
+
+; Same as before, but reversing the order of the inputs
+; RUN: llvm-lto2 %t2.bc %t1.bc -o %t.o -save-temps \
+; RUN:  -r %t1.bc,v,px \
+; RUN:  -r %t2.bc,v,px \
+; RUN:  -r %t1.bc,foo,px \
+; RUN:  -r %t2.bc,bar,px
+; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck %s --check-prefix=BOTH-PREVAILED2
+
+
+
 target triple = "x86_64-apple-macosx10.11.0"
 
 @v = common global i8 0, align 8
 
 ; LARGE-PREVAILED: @v = common global i16 0, align 8
 ; SMALL-PREVAILED: @v = common global [2 x i8] zeroinitializer, align 8
-; In this case the first was kept as external, but we created a new merged
-; common due to the second requiring a larger size:
-; NONE-PREVAILED1: @v = common global [2 x i8] zeroinitializer, align 8
-; NONE-PREVAILED2: @v = external global i16, align 8
+; BOTH-PREVAILED1: @v = common global i16 0, align 8
+; BOTH-PREVAILED2: common global [2 x i8] zeroinitializer, align 8
+; In this case the first is kept as external
+; NONE-PREVAILED1: @v = external global i8, align 8
+; NONE-PREVAILED2: @v = external global i16, align 4
 
 define i8 *@foo() {
  ret i8 *@v




More information about the llvm-commits mailing list