[PATCH] D153097: [RISCV] Make linker-relaxable instructions terminate MCDataFragment

Thu Jun 29 08:40:25 PDT 2023

barannikov88 added a comment.

In D153097#4459780 <https://reviews.llvm.org/D153097#4459780>, @asb wrote:

> I've stepped through the patch description and the code - both of which make sense to me. I'm not familiar enough with the code to be certain there's not an alternative way of achieving the same thing that would be better....but then I'd normally looking to @maskray for such suggestions :) LGTM, just left a minor query regarding tweaking a doc comment.

This was my main concern. The patch assumes there is only one possible fixup that may need linker relaxation, and only supported on ELF targets.
I can imagine that RISC-V could be supported by non-unix OS, or that more fixups needing linker relaxation can be added, possibly by other targets.
I hoped my concern could be addressed, but I might as well be nitpicking, as it can easily be addressed in the future.

================
Comment at: llvm/lib/MC/MCExpr.cpp:663
+    uint64_t SAOffset = SA.getOffset(), SBOffset = SB.getOffset();
     int64_t Displacement = SA.getOffset() - SB.getOffset();
     bool Reverse = false;
----------------
(nit) It could be slightly easier to follow if the Displacement was calculated after the fragments are sorted.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153097