[PATCH] D142879: [RISCV] Emit relocation for uleb128

Mon Jul 17 07:06:48 PDT 2023

kito-cheng marked an inline comment as done.
kito-cheng added inline comments.

================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:210
+  const MCExpr *A, *B;
+  if (!EnableULEB128Reloc || !requiresFixups(getContext(), Value, A, B))
+    return MCELFStreamer::emitULEB128Value(Value);
----------------
MaskRay wrote:
> kito-cheng wrote:
> > MaskRay wrote:
> > > evaluateAsAbsolute has now been correct (D153097). I am now mainly concerned with our inaccurate `requiresFixups` and whether there will be users rely on a possibly buggy behavior...
> > Did you have suggestion on this, I guess I am not really family enough around that :(
> I took quite some time today to investigate it.... See D155357
> 
> I think we need a custom fixup to represent the `fixup_riscv_set_uleb128` and `fixup_riscv_sub_uleb128` pair.
> 
> In `.handleAddSubRelocations`, split the fixup into 2 relocations.
Thanks, I am based on D155357 now :)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142879