[PATCH] D145687: [BOLT] Add minimal RISC-V 64-bit support

[Job Noorman via Phabricator via llvm-commits]

jobnoorman added inline comments.


================
Comment at: bolt/lib/Rewrite/RewriteInstance.cpp:2669-2675
+    } else if (ReferencedSymbol) {
+      // This might be a relocation for an ABS symbols like __global_pointer$ on
+      // RISC-V
+      ContainingBF->addRelocation(Rel.getOffset(), ReferencedSymbol,
+                                  Rel.getType(), 0,
+                                  cantFail(Symbol.getValue()));
+      return;
----------------
rafauler wrote:
> The new solution is triggering a lot more than the previous one, now we're definitely getting undefined symbols. In one X86 input we're getting these entries now being used to generate relocs:
> 
> 714106: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND symbol1
> 714107: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND symbol2
> 
> Unless we plan to define symbols in BOLT, which I can't think why we would do that, I think we should explicitly filter out undefined syms when adding relocations to functions.
Interesting! I found a way to check that the symbol is actually an ABS one.

Note that I wasn't able to reproduce this issue (I might've misunderstood/done something wrong though). If you're still seeing issues with this, could you maybe point me to an input file to reproduce it?


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

https://reviews.llvm.org/D145687