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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D153097#4442561 <https://reviews.llvm.org/D153097#4442561>, @nickdesaulniers wrote:

> Is it possible to tell which instructions are linker-relaxable? If so, perhaps fixing `MCExpr::evaluateAsAbsolute` to scan fragments for those is a better fix?

Thanks for taking a look.
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-elf.adoc#linker-relaxation has listed some instructions that are linker-relaxable (they generate a `R_RISCV_RELAX` relocation with the default `-mrelax`).

`AttemptToFoldSymbolOffsetDifference` is eventually called by `MCExpr::evaluateAsAbsolute`.
Fragment scanning should generally be avoided. Adding another fragment scan to `MCExpr::evaluateAsAbsolute` would increase complexity.
After intensive investigation, I conclude that `AttemptToFoldSymbolOffsetDifference`  is the place to fix...



================
Comment at: llvm/lib/MC/MCExpr.cpp:688
+      if (DF && DF->isLinkerRelaxable()) {
+        if (&*FI != FB || SBOffset != DF->getContents().size())
+          BBeforeRelax = true;
----------------
nickdesaulniers wrote:
> once set, we probably don't need to check these other conditions? maybe check `!BBeforeRelax` first, and `!AAfterRelax` below (L690)?
> 
> ```
> if (!BBeforeRelax && (&*FI != FB || ...))
>   ...
> ```
We could add `!BBeforeRelax && ` but I feel that this just makes the code slightly more complex without providing a performance improvement, so I think we may just afford redundant BBeforeRelax assignment...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153097