[PATCH] D125036: [RISCV] Alignment relaxation

Luís Marques via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 26 07:44:48 PDT 2022


luismarques added a comment.

In D125036#3526818 <https://reviews.llvm.org/D125036#3526818>, @MaskRay wrote:

> While I think there are still significant problems needing to address, it may work with quite a few application. It'd be nice if folks can check how this patch works without `-mno-relax`.

My testing hasn't revealed any problems so far. I have looked at the code but I haven't yet evaluated it as carefully as I would like.



================
Comment at: lld/ELF/Arch/RISCV.cpp:533-535
+// After one or more contractions and/or expansions of the address range that
+// rounds-up to the alignment boundary, the sequence of NOPs emitted by the
+// compiler could be corrupted. Repair by rewriting an optimal sequence of NOPs.
----------------
Please describe in the comment what you mean by a corrupted NOP sequence.


================
Comment at: lld/ELF/Arch/RISCV.cpp:542
+// if necessary, use a single 2-byte C.NOP to finish.
+void fillAdjustGaps() {
+  for (OutputSection *osec : outputSections)
----------------
add `static`?


================
Comment at: lld/ELF/Arch/RISCV.cpp:571-573
+// Execute a single relaxation pass. Return TRUE if we did something.  The
+// caller will repeatedly call this until the algorithm converges, as indicated
+// by a pass where nothing happens.
----------------
What expectations should we have about the convergence? Let's write some of that down. Either in this comment or in some place that provides a more global overview.


================
Comment at: lld/ELF/Arch/RISCV.cpp:585
+          relaxAlign(isec, r, delta, ranges);
+        // TODO(gkm): handle call/jump/load/store/addr-arithmetic relaxation
+      }
----------------
Nit: remove the user from the TODO.


================
Comment at: lld/test/ELF/riscv-relax-align-rvc.s:10
+#
+# Aligning the after the second `.balign 16` requires an overlapped copy.
+# Verify that no instructions are clobbered.
----------------
Typo: "the after".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125036



More information about the llvm-commits mailing list