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

[Sriraman Tallam via llvm-commits]

On Wed, Jan 17, 2018, 5:56 PM Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> Reid Kleckner <rnk at google.com> writes:
>
> >> Index: lib/Target/X86/X86ISelLowering.cpp
> >> > ===================================================================
> >> > --- lib/Target/X86/X86ISelLowering.cpp
> >> > +++ lib/Target/X86/X86ISelLowering.cpp
> >> > @@ -3732,11 +3732,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->getAvoidPLT()) {
> >> > +      OpFlags = Subtarget.classifyGlobalFunctionReference(nullptr,
> >> *Mod);
> >> > +    }
> >> >
> >> >      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()));
> >> > +    }
> >>
> >> The logic should be added to shouldAssumeDSOLocal.
> >>
> >
> > We might need to extend shouldAssumeDSOLocal to know that we are calling
> a
> > function. We only have an ExternalSymbol in this case, not a GlobalValue,
> > so GV there will be null.
>
> My memory in these areas is fuzzy. Do we never create a GV for a call to
> an intrinsic or do we lose it at some point in the codegen path? If it is
> lost, could we track it a bit longer?
>
> For this patch, is there any other case when GV is null? If that only
> happens when calling an intrinsic shouldAssumeDSOLocal can use that to
> know that it is a call.
>
> Maybe it can always return false if GV is null?
>
> There are 3 ways of calling a function:
>
> call foo
> call foo at plt
> call *foo at got
>
> if shouldAssumeDSOLocal returns true, we are supposed to use the first
> one. If it returns false, one of the last two.
>
> Now, "call "foo" is almost identical to "call foo at plt" as the linker can
> convert plt calls if the target is local. If the only case when GV is
> null is a call to an intrinsic, it is safe to return false.
>

It looks like that's the case. However it looks pretty ugly. Also if we do
decide to go down this path we need to figure out where to add the
nonlazybind attribute to the intrinsic.


> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180118/771ac15c/attachment.html>