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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/include/llvm/MC/MCFixup.h:28
+  FK_Data_6b,      ///< A six-bits fixup.
+  FK_Data_uleb128, ///< A uleb128 fixup.
+  FK_PCRel_1,      ///< A one-byte pc relative fixup.
----------------
We do not need this generic fixup.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp:164
     return ELF::R_RISCV_SET32;
   case RISCV::fixup_riscv_add_32:
     return ELF::R_RISCV_ADD32;
----------------
These `fixup_riscv_{set,add,sub}_*` from D103539 are unneeded. I did not put the cleanup into D155357 to minimize the changes in D155357 . I have now removed the unneeded fixup kinds in c8ed138c34ddb22610f18468c1b7938f9e2abae5.

I think you can just use `FirstLiteralRelocationKind + ELF::R_RISCV_SET_ULEB128` instead of adding new fixup kinds.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp:149
+  if (IntValue < 0)
+    report_fatal_error("Can't encoding negative value in uleb128 format.");
+
----------------
Prefer `getContext().reportError` for possible error paths. `report_fatal_error` should be avoided if possible.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVFixupKinds.h:110
+  // fixup corresponding to R_RISCV_SET_ULEB128 for local label assignment.
+  fixup_riscv_set_uleb128,
+  // fixup corresponding to R_RISCV_SUB_ULEB128 for local label assignment.
----------------
Unneeded. See my other comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142879