[X86] Call locally defined function directly for PIE

[hjl.tools]

Summary

There is no need to use PLT to call locally defined function for PIE
since locally defined function in PIE can't be preempted.

Fixes PR 24970.

[DavidKreitzer]

test/CodeGen/X86/pie-2.ll

1 ↗

(On Diff #36648)

This test is almost identical to pie-1.ll. Can you combine them?

[hjl.tools]

I tried putting

; LINUX-NOT: call{{l|q}} foo@PLT
; LINUX: call{{l|q}} foo

in the same file. But test passed even if llvm wasn't fixed. It seems that

; LINUX: call{{l|q}} foo

matches

call{{l|q}} foo@PLT

and cancels

; LINUX-NOT: call{{l|q}} foo@PLT

[DavidKreitzer]

How about something like this?

; LINUX: call{{l|q}} foo{{ |$}}

There are other alternatives as well. Just about anything would be better than replicating the entire test.

[hjl.tools]

Update to a single testcase with

; LINUX: call{{l|q}} foo{{$}}

[• rafael]

lib/Target/X86/X86ISelLowering.cpp

3314

This needs to be refactored to a helper function that is used in both places.

Should this really be isStrongDefinitionForLinker? If this is a weak_odr or linkonce_odr, can't we avoid the plt? Add a test for that one way or the other.

[hjl.tools]

I added a helper function, ClassifyGlobalFunctionReference, and used
it in both places. I was looking for a predicate to tell me if a symbol is
defined. The symbol can be weak or linkonce. Should I use
!isDeclarationForLinker instead?

[hjl.tools]

This seems to work:

diff --git a/lib/Target/X86/X86Subtarget.cpp b/lib/Target/X86/X86Subtarget.cpp
index d94e91b..34f88b9 100644
--- a/lib/Target/X86/X86Subtarget.cpp
+++ b/lib/Target/X86/X86Subtarget.cpp
@@ -154,8 +154,8 @@ ClassifyGlobalFunctionReference(const GlobalValue *GV,
   // we don't need to use the PLT - we can directly call it.
   if (isTargetELF() &&
       TM.getRelocationModel() == Reloc::PIC_ &&
-      !(TM.Options.PositionIndependentExecutable &&
-        GV->isStrongDefinitionForLinker()) &&
+      (!TM.Options.PositionIndependentExecutable ||
+       GV->isDeclarationForLinker()) &&
       GV->hasDefaultVisibility() && !GV->hasLocalLinkage()) {
     return X86II::MO_PLT;
   } else if (isPICStyleStubAny() &&

[• rafael]

This is still missing a testcase for other linkages (weak_odr and weak at least).

Why do we need to use a PLT for calling those?

Even for regular weak and linkonce, why do we need a plt? We know there is *a* definition in the executable. The static linker might resolve it to a definition in some other .o, but it will still be defined in the executable and cannot be preempted out of it.

lib/Target/X86/X86Subtarget.cpp

147

Don't duplicate comments from the .h file.

lib/Target/X86/X86Subtarget.h

504

Don't repeat the name in the comment

506

Function names should start with a lowercase letter.

test/CodeGen/X86/pie.ll

1

Drop the -check-prefix=LINUX and just use the default (CHECK)

10

You don't need the comments.

Simplify foo to return null

define void @foo() {

ret void

}

lib/Target/X86/X86FastISel.cpp

     unsigned CallOpc = Is64Bit ? X86::CALL64pcrel32 : X86::CALLpcrel32;
 
     // See if we need any target-specific flags on the GV operand.
-    unsigned char OpFlags = 0;
-
-    // On ELF targets, in both X86-64 and X86-32 mode, direct calls to
-    // external symbols most go through the PLT in PIC mode.  If the symbol
-    // has hidden or protected visibility, or if it is static or local, then
-    // we don't need to use the PLT - we can directly call it.
-    if (Subtarget->isTargetELF() &&
-        TM.getRelocationModel() == Reloc::PIC_ &&
-        GV->hasDefaultVisibility() && !GV->hasLocalLinkage()) {
-      OpFlags = X86II::MO_PLT;
-    } else if (Subtarget->isPICStyleStubAny() &&
-               !GV->isStrongDefinitionForLinker() &&
-               (!Subtarget->getTargetTriple().isMacOSX() ||
-                Subtarget->getTargetTriple().isMacOSXVersionLT(10, 5))) {
-      // PC-relative references to external symbols should go through $stub,
-      // unless we're building with the leopard linker or later, which
-      // automatically synthesizes these stubs.
-      OpFlags = X86II::MO_DARWIN_STUB;
-    }
+    unsigned char OpFlags
+        = Subtarget->ClassifyGlobalFunctionReference(GV, TM);
 
     MIB = BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(CallOpc));
     if (Symbol)

lib/Target/X86/X86ISelLowering.cpp

     // non-JIT mode.
     const GlobalValue *GV = G->getGlobal();
     if (!GV->hasDLLImportStorageClass()) {
-      unsigned char OpFlags = 0;
+      unsigned char OpFlags
+          = Subtarget->ClassifyGlobalFunctionReference(GV, DAG.getTarget());
       bool ExtraLoad = false;
       unsigned WrapperKind = ISD::DELETED_NODE;
-
-      // On ELF targets, in both X86-64 and X86-32 mode, direct calls to
-      // external symbols most go through the PLT in PIC mode.  If the symbol
-      // has hidden or protected visibility, or if it is static or local, then
-      // we don't need to use the PLT - we can directly call it.
-      if (Subtarget->isTargetELF() &&
-          DAG.getTarget().getRelocationModel() == Reloc::PIC_ &&
-          GV->hasDefaultVisibility() && !GV->hasLocalLinkage()) {
-        OpFlags = X86II::MO_PLT;
-      } else if (Subtarget->isPICStyleStubAny() &&
-                 !GV->isStrongDefinitionForLinker() &&
-                 (!Subtarget->getTargetTriple().isMacOSX() ||
-                  Subtarget->getTargetTriple().isMacOSXVersionLT(10, 5))) {
-        // PC-relative references to external symbols should go through $stub,
-        // unless we're building with the leopard linker or later, which
-        // automatically synthesizes these stubs.
-        OpFlags = X86II::MO_DARWIN_STUB;
-      } else if (Subtarget->isPICStyleRIPRel() && isa<Function>(GV) &&
-                 cast<Function>(GV)->hasFnAttribute(Attribute::NonLazyBind)) {
+      if (OpFlags == X86II::MO_NO_FLAG &&
+          Subtarget->isPICStyleRIPRel() && isa<Function>(GV) &&
+          cast<Function>(GV)->hasFnAttribute(Attribute::NonLazyBind)) {
         // If the function is marked as non-lazy, generate an indirect call
         // which loads from the GOT directly. This avoids runtime overhead
         // at the cost of eager binding (and one extra byte of encoding).

lib/Target/X86/X86Subtarget.h

   unsigned char ClassifyGlobalReference(const GlobalValue *GV,
                                         const TargetMachine &TM)const;
 
+  /// ClassifyGlobalFunctionReference - Classify a global function reference
+  /// for the current subtarget.
+  unsigned char ClassifyGlobalFunctionReference(const GlobalValue *GV,
+                                                const TargetMachine &TM)const;
+
   /// Classify a blockaddress reference for the current subtarget according to
   /// how we should reference it in a non-pcrel context.
   unsigned char ClassifyBlockAddressReference() const;

lib/Target/X86/X86Subtarget.cpp

   return X86II::MO_NO_FLAG;
 }
 
+/// Classify a global function reference for the current subtarget.
+unsigned char X86Subtarget::
+ClassifyGlobalFunctionReference(const GlobalValue *GV,
+                                const TargetMachine &TM) const {
+  // On ELF targets, in both X86-64 and X86-32 mode, direct calls to
+  // external symbols most go through the PLT in PIC mode.  If the symbol
+  // has hidden or protected visibility, or if it is static or local, then
+  // we don't need to use the PLT - we can directly call it.
+  if (isTargetELF() &&
+      TM.getRelocationModel() == Reloc::PIC_ &&
+      !(TM.Options.PositionIndependentExecutable &&
+        GV->isStrongDefinitionForLinker()) &&
+      GV->hasDefaultVisibility() && !GV->hasLocalLinkage()) {
+    return X86II::MO_PLT;
+  } else if (isPICStyleStubAny() &&
+             !GV->isStrongDefinitionForLinker() &&
+             (!getTargetTriple().isMacOSX() ||
+              getTargetTriple().isMacOSXVersionLT(10, 5))) {
+    // PC-relative references to external symbols should go through $stub,
+    // unless we're building with the leopard linker or later, which
+    // automatically synthesizes these stubs.
+    return X86II::MO_DARWIN_STUB;
+  }
+
+  return X86II::MO_NO_FLAG;
+}
 
 /// This function returns the name of a function which has an interface like
 /// the non-standard bzero function, if such a function exists on the

test/CodeGen/X86/pie.ll

+; RUN: llc < %s -mcpu=generic -mtriple=i686-linux-gnu -relocation-model=pic -asm-verbose=false -enable-pie | FileCheck %s -check-prefix=LINUX
+; RUN: llc < %s -mcpu=generic -mtriple=i686-linux-gnu -fast-isel -relocation-model=pic -asm-verbose=false -enable-pie | FileCheck %s -check-prefix=LINUX
+; RUN: llc < %s -mcpu=generic -mtriple=x86_64-linux-gnu -relocation-model=pic -asm-verbose=false -enable-pie | FileCheck %s -check-prefix=LINUX
+; RUN: llc < %s -mcpu=generic -mtriple=x86_64-linux-gnu -fast-isel -relocation-model=pic -asm-verbose=false -enable-pie | FileCheck %s -check-prefix=LINUX
+
+; LINUX:  call{{l|q}} foo{{$}}
+
+@glob = common global i32 0, align 4
+
+; Function Attrs: nounwind uwtable
+define i32 @foo() {
+entry:
+  %0 = load i32, i32* @glob, align 4
+  ret i32 %0
+}
+
+; Function Attrs: nounwind uwtable
+define void @bar() {
+entry:
+  %call = call i32 @foo()
+  store i32 %call, i32* @glob, align 4
+  ret void
+}
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 1, !"PIC Level", i32 2}