[PATCH] D92114: [X86] Don't emit R_X86_64_[REX_]GOTPCRELX for a GOT load with an offset

Fri Nov 27 00:17:59 PST 2020

jhenderson added a comment.

My only remaining concern is the comment.

================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp:401-402
+    unsigned FixupKind = [&]() {
+      // Enable relaxed relocation only for a MCSymbolRefExpr.  We cannot
+      // enabled relaxed relocation if an offset is present (e.g. x at GOTPCREL+4).
+      if (!(Disp.isExpr() && isa<MCSymbolRefExpr>(Disp.getExpr())))
----------------
MaskRay wrote:
> jhenderson wrote:
> > I think we should keep the old comment, at least in part, since it may not be obvious to a future developer what a "relaxed relocation form" actually means in practice. We can then just add this new comment after it, saying why we don't emit such a relocation for all situations.
> The old comment was about "movq loads". I moved it below.
The new comment below doesn't say anything about the linker being able "to eliminate some loads for GOT references which end up in the same linkage unit" which I think is useful information and should be kept somewhere.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92114/new/

https://reviews.llvm.org/D92114