From 30e8f83c84c5a302a559722fc0d2973dc3f425ee Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Tue, 1 Feb 2022 10:41:16 -0800 Subject: [PATCH] [GlobalOpt] Don't replace alias with aliasee if either alias/aliasee may be preemptible Generalize D99629 for ELF. A default visibility non-local symbol is preemptible in a -shared link. `isInterposable` is an insufficient condition. Moreover, a non-preemptible alias may be referenced in a sub constant expression which intends to lower to a PC-relative relocation. Replacing the alias with a preemptible aliasee may introduce a linker error. Respect dso_preemptable and suppress optimization to fix the abose issues. With the change, `alias = 345` will not be rewritten to use aliasee in a `-fpic` compile. ``` int aliasee; extern int alias __attribute__((alias("aliasee"), visibility("hidden"))); void foo() { alias = 345; } // intended to access the local copy ``` While here, refine the condition for the alias as well. For some binary formats like COFF, `isInterposable` is a sufficient condition. But I think canonicalization for the changed case has little advantage, so I don't bother to add the `Triple(M.getTargetTriple()).isOSBinFormatELF()` or `getPICLevel/getPIELevel` complexity. For instrumentations, it's recommended not to create aliases that refer to globals that have a weak linkage or is preemptible. However, the following is supported and the IR needs to handle such cases. ``` int aliasee __attribute__((weak)); extern int alias __attribute__((alias("aliasee"))); ``` There are other places where GlobalAlias isInterposable usage may need to be fixed. Reviewed By: rnk Differential Revision: https://reviews.llvm.org/D107249 --- llvm/docs/LangRef.rst | 5 ++++ llvm/lib/Transforms/IPO/GlobalOpt.cpp | 17 +++++++++--- .../GlobalOpt/2009-02-15-ResolveAlias.ll | 6 ++--- llvm/test/Transforms/GlobalOpt/alias-resolve.ll | 30 ++++++++++++++-------- .../GlobalOpt/alias-used-address-space.ll | 2 +- llvm/test/Transforms/GlobalOpt/alias-used.ll | 4 +-- 6 files changed, 43 insertions(+), 21 deletions(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 40f2320..920834b 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -894,6 +894,11 @@ some can only be checked when producing an object file: * No global value in the expression can be a declaration, since that would require a relocation, which is not possible. +* If either the alias or the aliasee may be replaced by a symbol outside the + module at link time or runtime, any optimization cannot replace the alias with + the aliasee, since the behavior may be different. The alias may be used as a + name guaranteed to point to the content in the current module. + .. _langref_ifunc: IFuncs diff --git a/llvm/lib/Transforms/IPO/GlobalOpt.cpp b/llvm/lib/Transforms/IPO/GlobalOpt.cpp index a29d4f6..1cb32e3 100644 --- a/llvm/lib/Transforms/IPO/GlobalOpt.cpp +++ b/llvm/lib/Transforms/IPO/GlobalOpt.cpp @@ -2225,6 +2225,13 @@ OptimizeGlobalAliases(Module &M, for (GlobalValue *GV : Used.used()) Used.compilerUsedErase(GV); + // Return whether GV is explicitly or implicitly dso_local and not replaceable + // by another definition in the current linkage unit. + auto IsModuleLocal = [](GlobalValue &GV) { + return !GlobalValue::isInterposableLinkage(GV.getLinkage()) && + (GV.isDSOLocal() || GV.isImplicitDSOLocal()); + }; + for (GlobalAlias &J : llvm::make_early_inc_range(M.aliases())) { // Aliases without names cannot be referenced outside this module. if (!J.hasName() && !J.isDeclaration() && !J.hasLocalLinkage()) @@ -2236,18 +2243,20 @@ OptimizeGlobalAliases(Module &M, } // If the alias can change at link time, nothing can be done - bail out. - if (J.isInterposable()) + if (!IsModuleLocal(J)) continue; Constant *Aliasee = J.getAliasee(); GlobalValue *Target = dyn_cast(Aliasee->stripPointerCasts()); // We can't trivially replace the alias with the aliasee if the aliasee is // non-trivial in some way. We also can't replace the alias with the aliasee - // if the aliasee is interposable because aliases point to the local - // definition. + // if the aliasee may be preemptible at runtime. On ELF, a non-preemptible + // alias can be used to access the definition as if preemption did not + // happen. // TODO: Try to handle non-zero GEPs of local aliasees. - if (!Target || Target->isInterposable()) + if (!Target || !IsModuleLocal(*Target)) continue; + Target->removeDeadConstantUsers(); // Make all users of the alias use the aliasee instead. diff --git a/llvm/test/Transforms/GlobalOpt/2009-02-15-ResolveAlias.ll b/llvm/test/Transforms/GlobalOpt/2009-02-15-ResolveAlias.ll index c297533..ce7d4fc 100644 --- a/llvm/test/Transforms/GlobalOpt/2009-02-15-ResolveAlias.ll +++ b/llvm/test/Transforms/GlobalOpt/2009-02-15-ResolveAlias.ll @@ -2,13 +2,13 @@ define internal void @f() { ; CHECK-NOT: @f( -; CHECK: define void @a +; CHECK: define dso_local void @a ret void } -@a = alias void (), void ()* @f +@a = dso_local alias void (), void ()* @f -define void @g() { +define hidden void @g() { call void() @a() ret void } diff --git a/llvm/test/Transforms/GlobalOpt/alias-resolve.ll b/llvm/test/Transforms/GlobalOpt/alias-resolve.ll index b4bee76..efac612 100644 --- a/llvm/test/Transforms/GlobalOpt/alias-resolve.ll +++ b/llvm/test/Transforms/GlobalOpt/alias-resolve.ll @@ -1,16 +1,19 @@ ; RUN: opt < %s -passes=globalopt -S | FileCheck %s @foo1 = alias void (), void ()* @foo2 +;; foo2 is dso_local and non-weak. Resolved. ; CHECK: @foo1 = alias void (), void ()* @bar2 -@foo2 = alias void(), void()* @bar1 -; CHECK: @foo2 = alias void (), void ()* @bar2 +@foo2 = dso_local alias void(), void()* @bar1 +;; bar1 is dso_local and non-weak. Resolved. +; CHECK: @foo2 = dso_local alias void (), void ()* @bar2 -@bar1 = alias void (), void ()* @bar2 -; CHECK: @bar1 = alias void (), void ()* @bar2 +@bar1 = dso_local alias void (), void ()* @bar2 +; CHECK: @bar1 = dso_local alias void (), void ()* @bar2 -@weak1 = weak alias void (), void ()* @bar2 -; CHECK: @weak1 = weak alias void (), void ()* @bar2 +@weak1 = weak dso_local alias void (), void ()* @bar2 +;; weak1 may be replaced with another definition in the linkage unit. Not resolved. +; CHECK: @weak1 = weak dso_local alias void (), void ()* @bar2 @bar4 = private unnamed_addr constant [2 x i8*] zeroinitializer @foo4 = weak_odr unnamed_addr alias i8*, getelementptr inbounds ([2 x i8*], [2 x i8*]* @bar4, i32 0, i32 1) @@ -19,10 +22,10 @@ @priva = private alias void (), void ()* @bar5 ; CHECK: @priva = private alias void (), void ()* @bar5 -define void @bar2() { +define dso_local void @bar2() { ret void } -; CHECK: define void @bar2() +; CHECK: define dso_local void @bar2() define weak void @bar5() { ret void @@ -32,27 +35,32 @@ define weak void @bar5() { define void @baz() { entry: call void @foo1() -; CHECK: call void @bar2() +;; foo1 is dso_preemptable. Not resolved. +; CHECK: call void @foo1() call void @foo2() +;; foo2 is dso_local and non-weak. Resolved. ; CHECK: call void @bar2() call void @bar1() +;; bar1 is dso_local and non-weak. Resolved. ; CHECK: call void @bar2() call void @weak1() +;; weak1 is weak. Not resolved. ; CHECK: call void @weak1() call void @priva() +;; priva has a local linkage. Resolved. ; CHECK: call void @priva() ret void } -@foo3 = alias void (), void ()* @bar3 +@foo3 = dso_local alias void (), void ()* @bar3 ; CHECK-NOT: bar3 define internal void @bar3() { ret void } -;CHECK: define void @foo3 +;CHECK: define dso_local void @foo3 diff --git a/llvm/test/Transforms/GlobalOpt/alias-used-address-space.ll b/llvm/test/Transforms/GlobalOpt/alias-used-address-space.ll index d119bfb..daf5acd 100644 --- a/llvm/test/Transforms/GlobalOpt/alias-used-address-space.ll +++ b/llvm/test/Transforms/GlobalOpt/alias-used-address-space.ll @@ -2,7 +2,7 @@ target datalayout = "p:32:32:32-p1:16:16:16" -@c = addrspace(1) global i8 42 +@c = hidden addrspace(1) global i8 42 @i = internal addrspace(1) global i8 42 diff --git a/llvm/test/Transforms/GlobalOpt/alias-used.ll b/llvm/test/Transforms/GlobalOpt/alias-used.ll index 73dd604..d8b7a76 100644 --- a/llvm/test/Transforms/GlobalOpt/alias-used.ll +++ b/llvm/test/Transforms/GlobalOpt/alias-used.ll @@ -1,6 +1,6 @@ ; RUN: opt < %s -passes=globalopt -S | FileCheck %s -@c = global i8 42 +@c = dso_local global i8 42 @i = internal global i8 42 ; CHECK: @ia = internal global i8 42 @@ -30,7 +30,7 @@ @ca = internal alias i8, i8* @c ; CHECK: @ca = internal alias i8, i8* @c -define void @f() { +define hidden void @f() { ret void } -- 2.7.4