[PATCH] D46677: [RISCV] Add R_RISCV_RELAX relocation to all possible relax candidates.

[Alex Bradbury via Phabricator via llvm-commits]

asb requested changes to this revision.
asb added inline comments.
This revision now requires changes to proceed.
Herald added subscribers: PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, rogfer01.


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp:193
   unsigned MIFrm = Desc.TSFlags & RISCVII::InstFormatMask;
+  bool RelaxCandidate = false;
 
----------------
It would make most sense to declare and initialise this alongside Expr/Kind/FixupKind, seeing as like those RelaxCandidate is relevant only for immediates.


================
Comment at: lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp:261
   ++MCNumFixups;
 
+  if (EnableRelax && RelaxCandidate) {
----------------
Suggest a comment here, something like:
"Ensure an R_RISCV_RELAX relocation will be emitted if linker relaxation is enabled and the current fixup will result in a relocation that may be relaxed."


================
Comment at: test/MC/RISCV/linker-relaxation.s:20
 # RELAX-FIXUP: fixup A - offset: 0, value: .L1, kind: fixup_riscv_branch
+
+lui t1, %hi(foo)
----------------
Check you add a check that R_RISCV_RELAX is not generated when -mattr=-relax is used?




https://reviews.llvm.org/D46677