Page MenuHomePhabricator

[X86] Call locally defined function directly for PIE
Needs ReviewPublic

Authored by on Oct 6 2015, 1:07 PM.



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.

Diff Detail


Event Timeline updated this revision to Diff 36648.Oct 6 2015, 1:07 PM retitled this revision from to [X86] Call locally defined function directly for PIE . updated this object. set the repository for this revision to rL LLVM. added a subscriber: llvm-commits.
DavidKreitzer added inline comments.Oct 8 2015, 9:34 AM
1 ↗(On Diff #36648)

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

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


call{{l|q}} foo@PLT

and cancels

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

DavidKreitzer edited edge metadata.Oct 9 2015, 6:19 AM

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. updated this revision to Diff 36958.Oct 9 2015, 9:28 AM edited edge metadata. removed rL LLVM as the repository for this revision.

Update to a single testcase with

; LINUX: call{{l|q}} foo{{$}} set the repository for this revision to rL LLVM.Oct 9 2015, 9:29 AM
rafael added inline comments.

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. updated this revision to Diff 38729.Oct 29 2015, 6:24 AM

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?

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() &&

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.


Don't duplicate comments from the .h file.


Don't repeat the name in the comment


Function names should start with a lowercase letter.


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


You don't need the comments.

Simplify foo to return null

define void @foo() {

ret void


dexonsmith resigned from this revision.Oct 6 2020, 3:44 PM