[PATCH] D38343: [X86] Provide the LSDA pointer with RIP relative addressing if necessary

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:20108
                                             Twine(MF.getFunctionNumber()));
-    return DAG.getNode(X86ISD::Wrapper, dl, VT, DAG.getMCSymbol(S, PtrVT));
+    // getGlobalWrapperKind isn't usable here.
+    return DAG.getNode(Subtarget.isPICStyleRIPRel() ? X86ISD::WrapperRIP
----------------
compnerd wrote:
> mstorsjo wrote:
> > compnerd wrote:
> > > Im not particularly fond of this comment.  Adding a comment that `getGlobalWrapperKind` is not suitable here is fine.  But please explain //why// it is unsuitable.  Also, it took me a moment to figure out that this is not actually dependent on the PIC mode of compilation, but rather to ensure that the generated code is PIC.  A comment to that effect would be nice.
> > So, something like this?
> > ```
> > // getGlobalWrapperKind can't be used here since we're in a static function and that requires an instance of X86TargetLowering.
> > // If we can use RIPRel addressing, prefer that since it doesn't cost anything extra, even if the build isn't required to be PIC.
> > ```
> Well, just because this function is static doesnt mean that we couldnt pass it a TLI.  It does technically cost a bit in code size.  I was thinking something like:
> 
>     // We use RIP relative addressing to generate PIC code as that is compatible across PIC and non-PIC code.  We would need to wire
>     // the X86TargetLowering to this function to use getGlobalWrapperKind.
> 
> Actually, how difficult would that be anyways?
Actually wasn't hard at all to make this a non-static function - that's probably cleaner.


https://reviews.llvm.org/D38343