[llvm] afb2e9f - [RISCV][MC] Add CLI option to disable branch relaxation

[Job Noorman via llvm-commits]

Author: Job Noorman
Date: 2023-07-28T10:42:05+02:00
New Revision: afb2e9f44c13101c7b22c99e080b0beb0cd5b01e

URL: https://github.com/llvm/llvm-project/commit/afb2e9f44c13101c7b22c99e080b0beb0cd5b01e
DIFF: https://github.com/llvm/llvm-project/commit/afb2e9f44c13101c7b22c99e080b0beb0cd5b01e.diff

LOG: [RISCV][MC] Add CLI option to disable branch relaxation

D154958 enables branch relaxation for unresolved symbols. This has an
interesting consequence for some LLD tests: branch relocations are
tested by using branches to undefined symbols and defining them, with
different values, on the LLD command line. These tests broke and there
doesn't seem to be an easy workaround: as far as I can tell, there is no
way to convince llvm-mc to emit a branch relocation to an undefined
symbol without branch relaxation kicking in.

This patch proposes to add a flag, `-riscv-asm-relax-branches=0`, to do
just that. The main purpose for this flag is for testing but it might be
seen as a first step to some kind of "strict" or WYSIWYG mode (i.e.,
what you give to the assembler is exactly what comes out). The need for
this has been mentioned in, for example, D108961. However, I suspect
there will be a lot of discussion around what exactly such a strict mode
would look like. Therefore, I gated this feature behind a CLI flag
instead of adding a new target feature.

Reviewed By: asb, MaskRay

Differential Revision: https://reviews.llvm.org/D155953

Added: 
    llvm/test/MC/RISCV/long-jump-disable-relax.s

Modified: 
    llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index 1b890fbe041a7e..6a46fffbb7b475 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -19,6 +19,7 @@
 #include "llvm/MC/MCObjectWriter.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/MCValue.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/EndianStream.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -27,6 +28,9 @@
 
 using namespace llvm;
 
+static cl::opt<bool> RelaxBranches("riscv-asm-relax-branches", cl::init(true),
+                                   cl::Hidden);
+
 std::optional<MCFixupKind> RISCVAsmBackend::getFixupKind(StringRef Name) const {
   if (STI.getTargetTriple().isOSBinFormatELF()) {
     unsigned Type;
@@ -144,6 +148,9 @@ bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCFixup &Fixup,
                                                    const MCRelaxableFragment *DF,
                                                    const MCAsmLayout &Layout,
                                                    const bool WasForced) const {
+  if (!RelaxBranches)
+    return false;
+
   int64_t Offset = int64_t(Value);
   unsigned Kind = Fixup.getTargetKind();
 
@@ -483,6 +490,8 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
     return UpperImm | ((LowerImm << 20) << 32);
   }
   case RISCV::fixup_riscv_rvc_jump: {
+    if (!isInt<12>(Value))
+      Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
     // Need to produce offset[11|4|9:8|10|6|7|3:1|5] from the 11-bit Value.
     unsigned Bit11  = (Value >> 11) & 0x1;
     unsigned Bit4   = (Value >> 4) & 0x1;
@@ -497,6 +506,8 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value,
     return Value;
   }
   case RISCV::fixup_riscv_rvc_branch: {
+    if (!isInt<9>(Value))
+      Ctx.reportError(Fixup.getLoc(), "fixup value out of range");
     // Need to produce offset[8|4:3], [reg 3 bit], offset[7:6|2:1|5]
     unsigned Bit8   = (Value >> 8) & 0x1;
     unsigned Bit7_6 = (Value >> 6) & 0x3;

diff  --git a/llvm/test/MC/RISCV/long-jump-disable-relax.s b/llvm/test/MC/RISCV/long-jump-disable-relax.s
new file mode 100644
index 00000000000000..815c2dfcec07aa
--- /dev/null
+++ b/llvm/test/MC/RISCV/long-jump-disable-relax.s
@@ -0,0 +1,47 @@
+## Test that long branches are not relaxed with -riscv-asm-relax-branches=0
+# RUN: split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+c \
+# RUN:       -riscv-asm-relax-branches=0 %t/pass.s \
+# RUN:     | llvm-objdump -dr -M no-aliases - \
+# RUN:     | FileCheck %t/pass.s
+# RUN: not llvm-mc -filetype=obj -triple=riscv64 -mattr=+c -o /dev/null \
+# RUN:       -riscv-asm-relax-branches=0 %t/fail.s 2>&1 \
+# RUN:     | FileCheck %t/fail.s
+
+#--- pass.s
+  .text
+test_undefined:
+## Branches to undefined symbols should not be relaxed
+# CHECK:      bne a0, a1, {{.*}}
+# CHECK-NEXT: R_RISCV_BRANCH foo
+   bne a0, a1, foo
+# CHECK:      c.beqz a0, {{.*}}
+# CHECK-NEXT: R_RISCV_RVC_BRANCH foo
+   c.beqz a0, foo
+# CHECK:      c.j {{.*}}
+# CHECK-NEXT: R_RISCV_RVC_JUMP foo
+   c.j foo
+
+## Branches to defined in-range symbols should work normally
+test_defined_in_range:
+# CHECK:      bne a0, a1, {{.*}} <bar>
+  bne a0, a1, bar
+# CHECK:      c.beqz a0, {{.*}} <bar>
+   c.beqz a0, bar
+# CHECK:      c.j {{.*}} <bar>
+   c.j bar
+bar:
+
+#--- fail.s
+  .text
+## Branches to defined out-of-range symbols should report an error
+test_defined_out_of_range:
+  bne a0, a1, 1f # CHECK: :[[#@LINE]]:3: error: fixup value out of range
+  .skip (1 << 12)
+1:
+  c.beqz a0, 1f # CHECK: :[[#@LINE]]:3: error: fixup value out of range
+  .skip (1 << 8)
+1:
+  c.j 1f # CHECK: :[[#@LINE]]:3: error: fixup value out of range
+  .skip (1 << 11)
+1: