Skip to content

Commit

Permalink
[RISCV][MC] Add CLI option to disable branch relaxation
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mtvec committed Jul 28, 2023
1 parent e76304d commit afb2e9f
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 0 deletions.
11 changes: 11 additions & 0 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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;
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
47 changes: 47 additions & 0 deletions llvm/test/MC/RISCV/long-jump-disable-relax.s
Original file line number Diff line number Diff line change
@@ -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:

0 comments on commit afb2e9f

Please sign in to comment.