[PATCH] D47507: [MC] [X86] Teach leaq _GLOBAL_OFFSET_TABLE(%rip), %r15 to use R_X86_64_GOTPC32 instead of R_X86_64_PC32

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp:355-356
     ImmOffset -= 4;
+    if (StartsWithGlobalOffsetTable(Expr) != GOT_None)
+      FixupKind = MCFixupKind(X86::reloc_global_offset_table);
+  }
----------------
echristo wrote:
> MaskRay wrote:
> > ruiu wrote:
> > > Is this the right place to add this code? There's code in this function that also set the same value to FixupKind. Can't you merge them?
> > When `%rip` is mentioned, r_addend should be relative to the PC of the **next** instruction thus the `ImmOffset -= 4` here (accounting for a 4-byte PC offset). This is different from the cases above (no PC is involved unless the magic _GLOBAL_OFFSET_TABLE_ appears, where r_addend should be relative to the PC of the **currenct** instruction ).
> > 
> > I think keeping them separate as is is not bad.
> Perhaps not right above, but I think the code might be clearer if we just add the change in Fixup right before this block and add the condition to this block.
> 
> Thoughts?
This block deals with 4-byte pc-relative `FixupKind`, while the 8-byte branch has been handled above.

 `_GLOBAL_OFFSET_TABLE_` can only be used in 4-byte or 8-byte `FixupKind`, the 1-byte and 2-byte branches below do not need this fix. Thus I think the current way organizes the logic well. Did I miss something?


Repository:
  rL LLVM

https://reviews.llvm.org/D47507