RISCV: honour `.option relax` in assembly

[compnerd]

Summary

We would lex the .option relax and effectively drop the request on the
floor. This is due to the fact that the configuration was set in the
assembler state, but the MC system only consults the static subtarget
information (and an additional bit?). Wire up the ASMStreamer to the
additional bit to ensure that the request is honoured.

It is important to note that the relaxation option is applied to the
entire file. As a result, the additional cases test that the rest of
the file is also processed accordingly.

[compnerd]

I don't understand the diff rendering on differential. The change is literally the 3 lines being added to RISCVTargetELFStreamer::emitDirectiveOptionRelax. The raw diff does not show any changes to the other lines around it.

[jrtc27]

Shouldn't this be covered by option-relax.s already though? Is that test wrong? Is it specifically because of the .option push? If so, just shove a .option push/.option pop pair around the whole file. This also feels like the wrong place to fix this; we already handle it for instructions in RISCVAsmParser::ParseInstruction, and that was a deliberate choice to make it lazy. In fact, I don't think there is a bug at all? If there are no instructions emitted subject to relaxation, the bit isn't set, because it's unnecessary, and if there are instructions emitted subject to relaxation, we set the bit. Am I missing something?

llvm/test/MC/RISCV/scoped-relaxation.s

2

Fix style to match other tests and remove redundant options. # is the canonical comment prefix for RISC-V, not //.

4

Don't indent any of these, it just makes the test look ugly.

[MaskRay]

llvm/test/MC/RISCV/scoped-relaxation.s

10

For llvm-readobj -r, in many cases I suggest that the { and } brace lines are checked as well to ensure the relocations are fully specified.

[compnerd]

Shouldn't this be covered by option-relax.s already though? Is that test wrong?

It isn't; the test doesn't cover any non-local symbols, so everything just happens to work out. I can certainly merge this into that file if you prefer.

[compnerd]

llvm/test/MC/RISCV/scoped-relaxation.s

4

I think that we can disagree then. I think that it is far more legible than everything being 0-column aligned. FWIW, most of the other backends do use the indentation (particularly the ones that I've worked with in LLVM in the past - ARM, ARM64, X86, and by extension X86_64).

[MaskRay]

llvm/test/MC/RISCV/scoped-relaxation.s

4

Personally I keep .globl/labels/etc at 0-column (top-level stuff) and .long/.word/etc at 2-column (function-level stuff).

[kito-cheng]

I would suggest few more line on testcase:

.option push
.option relax
call function #check R_RISCV_RELAX is here
.option pop
.option push
.option norelax #check no R_RISCV_RELAX
call function
.option pop

llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp

RISCVTargetELFStreamer::RISCVTargetELFStreamer(MCStreamer &S,
   case RISCVABI::ABI_Unknown:
     llvm_unreachable("Improperly initialised target ABI");
   }
 
   MCA.setELFHeaderEFlags(EFlags);
 }
 
 MCELFStreamer &RISCVTargetELFStreamer::getStreamer() {
-  return static_cast<MCELFStreamer &>(Streamer);
-}
-
-void RISCVTargetELFStreamer::emitDirectiveOptionPush() {}
-void RISCVTargetELFStreamer::emitDirectiveOptionPop() {}
-void RISCVTargetELFStreamer::emitDirectiveOptionPIC() {}
-void RISCVTargetELFStreamer::emitDirectiveOptionNoPIC() {}
-void RISCVTargetELFStreamer::emitDirectiveOptionRVC() {}
-void RISCVTargetELFStreamer::emitDirectiveOptionNoRVC() {}
-void RISCVTargetELFStreamer::emitDirectiveOptionRelax() {}
+  return static_cast<MCELFStreamer &>(Streamer);
+}
+
+void RISCVTargetELFStreamer::emitDirectiveOptionPush() {}
+void RISCVTargetELFStreamer::emitDirectiveOptionPop() {}
+void RISCVTargetELFStreamer::emitDirectiveOptionPIC() {}
+void RISCVTargetELFStreamer::emitDirectiveOptionNoPIC() {}
+void RISCVTargetELFStreamer::emitDirectiveOptionRVC() {}
+void RISCVTargetELFStreamer::emitDirectiveOptionNoRVC() {}
+void RISCVTargetELFStreamer::emitDirectiveOptionRelax() {
+  MCAssembler &MCA = getStreamer().getAssembler();
+  auto &MAB = static_cast<RISCVAsmBackend &>(MCA.getBackend());
+  MAB.setForceRelocs();
+}
 void RISCVTargetELFStreamer::emitDirectiveOptionNoRelax() {}
 
 void RISCVTargetELFStreamer::emitAttribute(unsigned Attribute, unsigned Value) {
   setAttributeItem(Attribute, Value, /*OverwriteExisting=*/true);
 }
 
 void RISCVTargetELFStreamer::emitTextAttribute(unsigned Attribute,
                                                StringRef String) {

llvm/test/MC/RISCV/scoped-relaxation.s

+# RUN: llvm-mc -mattr -relax -triple riscv64-unknown-none-elf -filetype obj %s -o - | llvm-readobj -d -r - | FileCheck %s
+
+.global function
+
+# Unrelaxed reference, this would normally fail, but the subsequent scoped
+# relaxation forces relaxation on the file.
+.dword function - .
+
+# CHECK: 0x0 R_RISCV_ADD64 function 0x0
+# CHECK: 0x0 R_RISCV_SUB64 - 0x0
+
+# Relaxed reference, this will resolve to a pair of `RISCV_ADD64` and
+# `RISCV_SUB64` relocation.
+.option push
+.option relax
+.dword function - .
+.option pop
+
+# CHECK: 0x8 R_RISCV_ADD64 function 0x0
+# CHECK: 0x8 R_RISCV_SUB64 - 0x0
+
+# Unrelaxed reference, this will resolve to a pair of `RISCV_ADD64` and
+# `RISCV_SUB64` relocation due to relaxation being sticky to the file.
+.option push
+.option norelax
+.dword function - .
+.option pop
+
+# CHECK: 0x10 R_RISCV_ADD64 function 0x0
+# CHECK: 0x10 R_RISCV_SUB64 - 0x0