[PATCH] D42216: Use New Module Metadata String "AvoidPLT" to avoid calls via PLT

[Rafael Avila de Espindola via llvm-commits]

Sriraman Tallam via Phabricator <reviews at reviews.llvm.org> writes:

> tmsriram updated this revision to Diff 131514.
> tmsriram added a comment.
>
> Rafael's change included and test case updated.  "Avoid PLT" changed to "RtLibUseGOT".
>
>
> https://reviews.llvm.org/D42216
>
> Files:
>   include/llvm/IR/Module.h
>   lib/IR/Module.cpp
>   lib/Target/X86/X86ISelLowering.cpp
>   test/CodeGen/X86/no-plt.ll
>
> Index: test/CodeGen/X86/no-plt.ll
> ===================================================================
> --- test/CodeGen/X86/no-plt.ll
> +++ test/CodeGen/X86/no-plt.ll
> @@ -3,7 +3,23 @@
>  ; RUN: llc < %s -mcpu=generic -mtriple=x86_64-linux-gnu \
>  ; RUN:   | FileCheck -check-prefix=X64 %s
>  
> -define i32 @main() #0 {
> +; Function Attrs: noinline nounwind uwtable
> +define void @memset_call(i8* nocapture %a, i8 %c, i32 %n) local_unnamed_addr #0 {
> +; X64: callq *memset at GOTPCREL(%rip)
> +entry:
> +  call void @llvm.memset.p0i8.i32(i8* %a, i8 %c, i32 %n, i1 false)
> +  ret void
> +}
> +
> +; Function Attrs: noinline nounwind uwtable
> +define void @memcpy_call(i8* nocapture %a, i8* nocapture readonly %b, i64 %n) local_unnamed_addr #0 {
> +; X64: callq *memcpy at GOTPCREL(%rip)
> +entry:
> +  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 %n, i32 1, i1 false)
> +  ret void
> +}
> +
> +define i32 @main() #1 {
>  ; X64: callq *_Z3foov at GOTPCREL(%rip)
>  ; X64: callq _Z3barv
>  ; X64: callq _Z3bazv
> @@ -18,13 +34,21 @@
>  }
>  
>  ; Function Attrs: nonlazybind
> -declare i32 @_Z3foov() #1
> +declare i32 @_Z3foov() #2
>  
> -declare i32 @_Z3barv() #2
> +declare i32 @_Z3barv() #3
>  
>  ; Function Attrs: nonlazybind
> -declare hidden i32 @_Z3bazv() #3
> +declare hidden i32 @_Z3bazv() #4
> +
> +; Function Attrs: argmemonly nounwind
> +declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture writeonly, i8* nocapture readonly, i64, i32, i1)
> +; Function Attrs: argmemonly nounwind
> +declare void @llvm.memset.p0i8.i32(i8* nocapture, i8, i32, i1) #0
> +
>  
> +attributes #2 = { nonlazybind }
> +attributes #4 = { nonlazybind }
>  
> -attributes #1 = { nonlazybind }
> -attributes #3 = { nonlazybind }
> +!llvm.module.flags = !{!1}
> +!1 = !{i32 7, !"RtLibUseGOT", i32 1}
> Index: lib/Target/X86/X86ISelLowering.cpp
> ===================================================================
> --- lib/Target/X86/X86ISelLowering.cpp
> +++ lib/Target/X86/X86ISelLowering.cpp
> @@ -3756,11 +3756,23 @@
>      }
>    } else if (ExternalSymbolSDNode *S = dyn_cast<ExternalSymbolSDNode>(Callee)) {
>      const Module *Mod = DAG.getMachineFunction().getFunction().getParent();
> -    unsigned char OpFlags =
> -        Subtarget.classifyGlobalFunctionReference(nullptr, *Mod);
> +    // If PLT must be avoided then the call should be via GOTPCREL.
> +    unsigned char OpFlags = X86II::MO_GOTPCREL;
> +
> +    if (!Mod->getRtLibUseGOT()) {
> +      OpFlags = Subtarget.classifyGlobalFunctionReference(nullptr, *Mod);
> +    }

This logic should be in X86Subtarget::classifyGlobalFunctionReference, no?

>      Callee = DAG.getTargetExternalSymbol(
>          S->getSymbol(), getPointerTy(DAG.getDataLayout()), OpFlags);
> +
> +    if (OpFlags == X86II::MO_GOTPCREL) {
> +      Callee = DAG.getNode(X86ISD::WrapperRIP, dl,
> +          getPointerTy(DAG.getDataLayout()), Callee);
> +      Callee = DAG.getLoad(
> +          getPointerTy(DAG.getDataLayout()), dl, DAG.getEntryNode(), Callee,
> +          MachinePointerInfo::getGOT(DAG.getMachineFunction()));
> +    }
>    } else if (Subtarget.isTarget64BitILP32() &&
>               Callee->getValueType(0) == MVT::i32) {
>      // Zero-extend the 32-bit Callee address into a 64-bit according to x32 ABI
> Index: lib/IR/Module.cpp
> ===================================================================
> --- lib/IR/Module.cpp
> +++ lib/IR/Module.cpp
> @@ -510,6 +510,19 @@
>    OwnedMemoryBuffer = std::move(MB);
>  }
>  
> +bool Module::getRtLibUseGOT() const {
> +  auto *Val = cast_or_null<ConstantAsMetadata>(getModuleFlag("RtLibUseGOT"));
> +
> +  if (Val && (cast<ConstantInt>(Val->getValue())->getZExtValue() > 0))
> +    return true;
> +
> +  return false;
> +}

This is just

return Val && (cast<ConstantInt>(Val->getValue())->getZExtValue() > 0);

> +
> +void Module::setRtLibUseGOT() {
> +  addModuleFlag(ModFlagBehavior::Max, "RtLibUseGOT", 1);
> +}

Since this is a module flag, please add a llvm-link test.

>  GlobalVariable *llvm::collectUsedGlobalVariables(
>      const Module &M, SmallPtrSetImpl<GlobalValue *> &Set, bool CompilerUsed) {
>    const char *Name = CompilerUsed ? "llvm.compiler.used" : "llvm.used";
> Index: include/llvm/IR/Module.h
> ===================================================================
> --- include/llvm/IR/Module.h
> +++ include/llvm/IR/Module.h
> @@ -838,6 +838,16 @@
>    Metadata *getProfileSummary();
>    /// @}
>  
> +  /// @name Utility functions for querying and setting if PLT must be avoided.

I don't think you need @name.


> +  /// \brief Returns true if PLT should be avoided for RTLib calls.
> +  bool getRtLibUseGOT() const;
> +
> +  /// \brief Set that PLT should be avoid for RTLib calls.
> +  void setRtLibUseGOT();
> +  /// @}


What is the "/// @}"?

You don't need \brief, it can be implicit.

Cheers,
Rafael