[PATCH] D13481: [X86] Call locally defined function directly for PIE

[Rafael Ávila de Espíndola via llvm-commits]

rafael added a comment.

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.


================
Comment at: lib/Target/X86/X86Subtarget.cpp:147
@@ -146,1 +146,3 @@
 
+/// Classify a global function reference for the current subtarget.
+unsigned char X86Subtarget::
----------------
Don't duplicate comments from the .h file.

================
Comment at: lib/Target/X86/X86Subtarget.h:504
@@ -503,1 +503,3 @@
 
+  /// ClassifyGlobalFunctionReference - Classify a global function reference
+  /// for the current subtarget.
----------------
Don't repeat the name in the comment

================
Comment at: lib/Target/X86/X86Subtarget.h:506
@@ +505,3 @@
+  /// for the current subtarget.
+  unsigned char ClassifyGlobalFunctionReference(const GlobalValue *GV,
+                                                const TargetMachine &TM)const;
----------------
Function names should start with a lowercase letter.

================
Comment at: test/CodeGen/X86/pie.ll:1
@@ +1,2 @@
+; 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
----------------
Drop the -check-prefix=LINUX and just use the default (CHECK)

================
Comment at: test/CodeGen/X86/pie.ll:10
@@ +9,3 @@
+
+; Function Attrs: nounwind uwtable
+define i32 @foo() {
----------------
You don't need the comments.

Simplify foo to return null

define void @foo() {
  ret void
}


Repository:
  rL LLVM

http://reviews.llvm.org/D13481