[llvm] r309278 - ThinLTO: Don't import aliases of any kind (even linkonce_odr)

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 27 08:09:06 PDT 2017


Author: dblaikie
Date: Thu Jul 27 08:09:06 2017
New Revision: 309278

URL: http://llvm.org/viewvc/llvm-project?rev=309278&view=rev
Log:
ThinLTO: Don't import aliases of any kind (even linkonce_odr)

Summary:
Until a more advanced version of importing can be implemented for
aliases (one that imports an alias as an available_externally definition
of the aliasee), skip the narrow subset of cases that was possible but
came at a cost: aliases of linkonce_odr functions could be imported
because the linkonce_odr function could be safely duplicated from the
source module. This came/comes at the cost of not being able to 'home'
imported linkonce functions (they had to be emitted linkonce_odr in all
the destination modules (even if they weren't used by an alias) rather
than as available_externally - causing extra object size).

Tangentially, this also was the only reason ThinLTO would emit multiple
CUs in to the resulting DWARF - which happens to be a problem for
Fission (there's a fix for this in GDB but not released yet, etc).
(actually it's not the only reason - but I'm sending a patch to fix the
other reason shortly)

There's no reason to believe this particularly narrow alias importing
was especially/meaningfully important, only that it was /possible/ to
implement in this way. When a more general solution is done, it should
still satisfy the DWARF concerns above, since the import will still be
available_externally, and thus not create extra CUs.

Since now all aliases are treated the same, I removed/simplified some
test cases since they were testing corner cases where there are no
longer any corners.

Reviewers: tejohnson, mehdi_amini

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

Removed:
    llvm/trunk/test/ThinLTO/X86/select_right_alias_definition.ll
Modified:
    llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
    llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp
    llvm/trunk/test/Linker/funcimport.ll
    llvm/trunk/test/ThinLTO/X86/alias_import.ll
    llvm/trunk/test/Transforms/FunctionImport/funcimport.ll

Modified: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=309278&r1=309277&r2=309278&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp Thu Jul 27 08:09:06 2017
@@ -132,16 +132,11 @@ selectCallee(const ModuleSummaryIndex &I
         if (GlobalValue::isInterposableLinkage(GVSummary->linkage()))
           // There is no point in importing these, we can't inline them
           return false;
-        if (auto *AS = dyn_cast<AliasSummary>(GVSummary)) {
-          GVSummary = &AS->getAliasee();
-          // Alias can't point to "available_externally". However when we import
-          // linkOnceODR the linkage does not change. So we import the alias
-          // and aliasee only in this case.
+        if (auto *AS = dyn_cast<AliasSummary>(GVSummary))
+          // Aliases can't point to "available_externally".
           // FIXME: we should import alias as available_externally *function*,
-          // the destination module does need to know it is an alias.
-          if (!GlobalValue::isLinkOnceODRLinkage(GVSummary->linkage()))
-            return false;
-        }
+          // the destination module does not need to know it is an alias.
+          return false;
 
         auto *Summary = cast<FunctionSummary>(GVSummary);
 
@@ -227,16 +222,11 @@ static void computeImportForFunction(
       DEBUG(dbgs() << "ignored! No qualifying callee with summary found.\n");
       continue;
     }
-    // "Resolve" the summary, traversing alias,
-    const FunctionSummary *ResolvedCalleeSummary;
-    if (isa<AliasSummary>(CalleeSummary)) {
-      ResolvedCalleeSummary = cast<FunctionSummary>(
-          &cast<AliasSummary>(CalleeSummary)->getAliasee());
-      assert(
-          GlobalValue::isLinkOnceODRLinkage(ResolvedCalleeSummary->linkage()) &&
-          "Unexpected alias to a non-linkonceODR in import list");
-    } else
-      ResolvedCalleeSummary = cast<FunctionSummary>(CalleeSummary);
+
+    // "Resolve" the summary
+    assert(!isa<AliasSummary>(CalleeSummary) &&
+           "Unexpected alias in import list");
+    const auto *ResolvedCalleeSummary = cast<FunctionSummary>(CalleeSummary);
 
     assert(ResolvedCalleeSummary->instCount() <= NewThreshold &&
            "selectCallee() didn't honor the threshold");

Modified: llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp?rev=309278&r1=309277&r2=309278&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp Thu Jul 27 08:09:06 2017
@@ -23,19 +23,13 @@ using namespace llvm;
 bool FunctionImportGlobalProcessing::doImportAsDefinition(
     const GlobalValue *SGV, SetVector<GlobalValue *> *GlobalsToImport) {
 
-  // For alias, we tie the definition to the base object. Extract it and recurse
-  if (auto *GA = dyn_cast<GlobalAlias>(SGV)) {
-    if (GA->isInterposable())
-      return false;
-    const GlobalObject *GO = GA->getBaseObject();
-    if (!GO->hasLinkOnceODRLinkage())
-      return false;
-    return FunctionImportGlobalProcessing::doImportAsDefinition(
-        GO, GlobalsToImport);
-  }
   // Only import the globals requested for importing.
   if (GlobalsToImport->count(const_cast<GlobalValue *>(SGV)))
     return true;
+
+  assert(!isa<GlobalAlias>(SGV) &&
+         "Unexpected global alias in the import list.");
+
   // Otherwise no.
   return false;
 }
@@ -132,8 +126,10 @@ FunctionImportGlobalProcessing::getLinka
     return SGV->getLinkage();
 
   switch (SGV->getLinkage()) {
+  case GlobalValue::LinkOnceAnyLinkage:
+  case GlobalValue::LinkOnceODRLinkage:
   case GlobalValue::ExternalLinkage:
-    // External defnitions are converted to available_externally
+    // External and linkonce definitions are converted to available_externally
     // definitions upon import, so that they are available for inlining
     // and/or optimization, but are turned into declarations later
     // during the EliminateAvailableExternally pass.
@@ -150,12 +146,6 @@ FunctionImportGlobalProcessing::getLinka
     // An imported available_externally declaration stays that way.
     return SGV->getLinkage();
 
-  case GlobalValue::LinkOnceAnyLinkage:
-  case GlobalValue::LinkOnceODRLinkage:
-    // These both stay the same when importing the definition.
-    // The ThinLTO pass will eventually force-import their definitions.
-    return SGV->getLinkage();
-
   case GlobalValue::WeakAnyLinkage:
     // Can't import weak_any definitions correctly, or we might change the
     // program semantics, since the linker will pick the first weak_any

Modified: llvm/trunk/test/Linker/funcimport.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/funcimport.ll?rev=309278&r1=309277&r2=309278&view=diff
==============================================================================
--- llvm/trunk/test/Linker/funcimport.ll (original)
+++ llvm/trunk/test/Linker/funcimport.ll Thu Jul 27 08:09:06 2017
@@ -59,11 +59,10 @@
 ; IMPORTGLOB4-DAG: define available_externally void @globalfunc1
 ; IMPORTGLOB4-DAG: declare void @weakalias
 
-; An alias to an imported function is imported as alias if the function is not
-; available_externally.
+; An alias is never imported.
 ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc -import=linkoncefunc:%t.bc -S | FileCheck %s --check-prefix=IMPORTGLOB5
-; IMPORTGLOB5-DAG: linkoncealias = alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
-; IMPORTGLOB5-DAG: define linkonce_odr void @linkoncefunc()
+; IMPORTGLOB5-NOT: @linkoncealias
+; IMPORTGLOB5-DAG: define available_externally void @linkoncefunc()
 
 ; Ensure that imported static variable and function references are correctly
 ; promoted and renamed (including static constant variable).

Modified: llvm/trunk/test/ThinLTO/X86/alias_import.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/alias_import.ll?rev=309278&r1=309277&r2=309278&view=diff
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/alias_import.ll (original)
+++ llvm/trunk/test/ThinLTO/X86/alias_import.ll Thu Jul 27 08:09:06 2017
@@ -2,13 +2,11 @@
 ; RUN: opt -module-summary %p/Inputs/alias_import.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc %t2.bc
 ; RUN: llvm-lto -thinlto-action=promote -thinlto-index %t.index.bc %t2.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=PROMOTE
-; RUN: llvm-lto -thinlto-action=promote -thinlto-index %t.index.bc %t2.bc -o - | llvm-lto -thinlto-action=internalize -thinlto-index %t.index.bc -thinlto-module-id=%t2.bc - -o - | llvm-dis -o - | FileCheck %s --check-prefix=PROMOTE-INTERNALIZE
 ; RUN: llvm-lto -thinlto-action=import -thinlto-index %t.index.bc %t1.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=IMPORT
 
-;
-; Alias can't point to "available_externally", so we can only import an alias
-; when we can import the aliasee with a linkage that won't be
-; available_externally, i.e linkOnceODR. (FIXME this limitation could be lifted)
+; Alias can't point to "available_externally", so they cannot be imported for
+; now. This could be implemented by importing the alias as an
+; available_externally definition copied from the aliasee's body.
 ; PROMOTE-DAG: @globalfuncAlias = alias void (...), bitcast (void ()* @globalfunc to void (...)*)
 ; PROMOTE-DAG: @globalfuncWeakAlias = weak alias void (...), bitcast (void ()* @globalfunc to void (...)*)
 ; PROMOTE-DAG: @globalfuncLinkonceAlias = weak alias void (...), bitcast (void ()* @globalfunc to void (...)*)
@@ -34,16 +32,12 @@
 ; PROMOTE-DAG: @weakODRfuncLinkonceAlias = weak alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
 ; PROMOTE-DAG: @weakODRfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
 ; PROMOTE-DAG: @weakODRfuncLinkonceODRAlias = weak_odr alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
-
-; Only alias to LinkonceODR aliasee can be imported
 ; PROMOTE-DAG: @linkonceODRfuncAlias = alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
 ; PROMOTE-DAG: @linkonceODRfuncWeakAlias = weak alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
 ; PROMOTE-DAG: @linkonceODRfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
-; Amongst these that are imported, check that we promote only linkonce->weak
 ; PROMOTE-DAG: @linkonceODRfuncLinkonceAlias = weak alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
 ; PROMOTE-DAG: @linkonceODRfuncLinkonceODRAlias = weak_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
 
-; These will be imported, check the linkage/renaming after promotion
 ; PROMOTE-DAG: define void @globalfunc()
 ; PROMOTE-DAG: define internal void @internalfunc()
 ; PROMOTE-DAG: define weak_odr void @linkonceODRfunc()
@@ -51,13 +45,12 @@
 ; PROMOTE-DAG: define weak void @linkoncefunc()
 ; PROMOTE-DAG: define weak void @weakfunc()
 
-; On the import side now, verify that aliases to a linkonce_odr are imported, but the weak/linkonce (we can't inline them)
+; On the import side now, verify that aliases are not imported
 ; IMPORT-DAG:  declare void @linkonceODRfuncWeakAlias
-; IMPORT-DAG: declare void @linkonceODRfuncLinkonceAlias
-; IMPORT-DAG:  @linkonceODRfuncAlias = alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
-; IMPORT-DAG:  @linkonceODRfuncWeakODRAlias = alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
-; IMPORT-DAG:  @linkonceODRfuncLinkonceODRAlias = linkonce_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
-; IMPORT-DAG:  define linkonce_odr void @linkonceODRfunc()
+; IMPORT-DAG:  declare void @linkonceODRfuncLinkonceAlias
+; IMPORT-DAG:  declare void @linkonceODRfuncAlias
+; IMPORT-DAG:  declare void @linkonceODRfuncWeakODRAlias
+; IMPORT-DAG:  declare void @linkonceODRfuncLinkonceODRAlias
 
 
 ; On the import side, these aliases are not imported (they don't point to a linkonce_odr)
@@ -86,46 +79,11 @@
 ; IMPORT-DAG: declare void @weakfuncLinkonceAlias()
 ; IMPORT-DAG: declare void @weakfuncWeakODRAlias()
 ; IMPORT-DAG: declare void @weakfuncLinkonceODRAlias()
-
-; Promotion + internalization should internalize all of these, except for aliases of
-; linkonce_odr functions, because alias to linkonce_odr are the only aliases we will
-; import (see selectCallee() in FunctionImport.cpp).
-; PROMOTE-INTERNALIZE-DAG: @globalfuncAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @globalfuncWeakAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @globalfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @globalfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @globalfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @globalfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @internalfuncAlias = internal alias void (...), bitcast (void ()* @internalfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @internalfuncWeakAlias = internal alias void (...), bitcast (void ()* @internalfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @internalfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @internalfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @internalfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @internalfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @internalfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @internalfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncAlias = alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncWeakAlias = internal alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncWeakODRAlias = weak_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @linkonceODRfuncLinkonceODRAlias = weak_odr alias void (...), bitcast (void ()* @linkonceODRfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @weakODRfuncAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @weakODRfuncWeakAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @weakODRfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @weakODRfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @weakODRfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @weakODRfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @linkoncefuncAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @linkoncefuncWeakAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @linkoncefuncLinkonceAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @linkoncefuncWeakODRAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @linkoncefuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @linkoncefunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @weakfuncAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @weakfuncWeakAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @weakfuncLinkonceAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @weakfuncWeakODRAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: @weakfuncLinkonceODRAlias = internal alias void (...), bitcast (void ()* @weakfunc to void (...)*)
-; PROMOTE-INTERNALIZE-DAG: define internal void @globalfunc()
-; PROMOTE-INTERNALIZE-DAG: define internal void @internalfunc()
-; PROMOTE-INTERNALIZE-DAG: define internal void @linkonceODRfunc()
-; PROMOTE-INTERNALIZE-DAG: define internal void @weakODRfunc()
-; PROMOTE-INTERNALIZE-DAG: define internal void @linkoncefunc()
-; PROMOTE-INTERNALIZE-DAG: define internal void @weakfunc()
+; IMPORT-DAG: declare void @linkonceODRfuncAlias()
+; IMPORT-DAG: declare void @linkonceODRfuncWeakAlias()
+; IMPORT-DAG: declare void @linkonceODRfuncWeakODRAlias()
+; IMPORT-DAG: declare void @linkonceODRfuncLinkonceAlias()
+; IMPORT-DAG: declare void @linkonceODRfuncLinkonceODRAlias()
 
 define i32 @main() #0 {
 entry:

Removed: llvm/trunk/test/ThinLTO/X86/select_right_alias_definition.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/select_right_alias_definition.ll?rev=309277&view=auto
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/select_right_alias_definition.ll (original)
+++ llvm/trunk/test/ThinLTO/X86/select_right_alias_definition.ll (removed)
@@ -1,27 +0,0 @@
-; RUN: opt -module-summary %s -o %t_main.bc
-; RUN: opt -module-summary %p/Inputs/select_right_alias_definition1.ll -o %t1.bc
-; RUN: opt -module-summary %p/Inputs/select_right_alias_definition2.ll -o %t2.bc
-
-; Make sure that we always select the right definition for alia foo, whatever
-; order the files are linked in.
-
-; Try with one order
-; RUN: llvm-lto -thinlto-action=thinlink -o %t.index1.bc %t_main.bc %t1.bc %t2.bc
-; RUN: llvm-lto -thinlto-action=import -thinlto-index %t.index1.bc %t_main.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=IMPORT
-
-; Try with the other order (reversing %t1.bc and %t2.bc)
-; RUN: llvm-lto -thinlto-action=thinlink -o %t.index2.bc %t_main.bc %t2.bc %t1.bc
-; RUN: llvm-lto -thinlto-action=import -thinlto-index %t.index2.bc %t_main.bc -o - | llvm-dis -o - | FileCheck %s --check-prefix=IMPORT
-
-; IMPORT: @foo = alias i32 (...), bitcast (i32 ()* @foo2 to i32 (...)*)
-; IMPORT: define linkonce_odr i32 @foo2() {
-; IMPORT-NEXT:  %ret = add i32 42, 42
-; IMPORT-NEXT:  ret i32 %ret
-; IMPORT-NEXT: }
-
-declare i32 @foo()
-
-define i32 @main() {
-    %ret = call i32 @foo()
-    ret i32 %ret
-}
\ No newline at end of file

Modified: llvm/trunk/test/Transforms/FunctionImport/funcimport.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionImport/funcimport.ll?rev=309278&r1=309277&r2=309278&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/FunctionImport/funcimport.ll (original)
+++ llvm/trunk/test/Transforms/FunctionImport/funcimport.ll Thu Jul 27 08:09:06 2017
@@ -40,12 +40,9 @@ declare void @weakalias(...) #1
 ; CHECK-DAG: declare void @analias
 declare void @analias(...) #1
 
-; Aliases import the aliasee function
+; Aliases are not imported
 declare void @linkoncealias(...) #1
-; INSTLIMDEF-DAG: Import linkoncealias
-; INSTLIMDEF-DAG: Import linkoncefunc
-; CHECK-DAG: define linkonce_odr void @linkoncefunc()
-; CHECK-DAG: @linkoncealias = alias void (...), bitcast (void ()* @linkoncefunc to void (...)*
+; CHECK-DAG: declare void @linkoncealias(...)
 
 ; INSTLIMDEF-DAG: Import referencestatics
 ; INSTLIMDEF-DAG: define available_externally i32 @referencestatics(i32 %i) !thinlto_src_module !0 {
@@ -108,7 +105,7 @@ declare void @linkoncefunc2(...) #1
 declare void @variadic(...)
 
 ; INSTLIMDEF-DAG: Import globalfunc2
-; INSTLIMDEF-DAG: 13 function-import - Number of functions imported
+; INSTLIMDEF-DAG: 11 function-import - Number of functions imported
 ; CHECK-DAG: !0 = !{!"{{.*}}/Inputs/funcimport.ll"}
 
 ; The actual GUID values will depend on path to test.




More information about the llvm-commits mailing list