[PATCH] D73211: [RISCV] Fix evaluating %pcrel_lo against global and weak symbols

Wed Jan 22 17:23:17 PST 2020

jrtc27 marked an inline comment as done.
jrtc27 added inline comments.

================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:323
+
+  if (shouldForceRelocation(Asm, Fixup, Target)) {
+    WasForced = true;
----------------
jrtc27 wrote:
> efriedma wrote:
> > `shouldForceRelocation(Asm, AUIPCFixup, AUIPCTarget)`?  (I think the difference is significant; please make sure we have a testcase that covers it.)
> It currently doesn't matter, the intent was only to pick up the FeatureRelax etc code path. But you're right, if I pass AUIPCFixup, things are better, and I can delete the WasForced case statements above (validating it's a valid fixup should already have happened in getPCRelHiFixup).
After removing the big switch earlier in the function, this is caught by at least one test in `relocations.s` which uses the GOT (and I assume also the TLS-related relocations later in the file), and it is now using the correct (AUIPC) fixup. It also uses the correct target, although that is not currently consulted so it does not matter.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73211