[llvm] 856745c - [RISCV][MC] Relax conditional branches to unresolved symbols
Job Noorman via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 28 01:56:04 PDT 2023
Author: Job Noorman
Date: 2023-07-28T10:55:58+02:00
New Revision: 856745c5ebe5e3da35c4f08bc6090e2d348ea048
URL: https://github.com/llvm/llvm-project/commit/856745c5ebe5e3da35c4f08bc6090e2d348ea048
DIFF: https://github.com/llvm/llvm-project/commit/856745c5ebe5e3da35c4f08bc6090e2d348ea048.diff
LOG: [RISCV][MC] Relax conditional branches to unresolved symbols
D108961 introduced relaxation for out-of-range conditional branches.
However, relaxation was only performed when the branch target could be
resolved. I believe this has two undesired consequences:
- `b<cc> ... foo`, where `foo` is undefined, would not be relaxed
although there is no guarantee the offset to `foo` will fit;
- Conditional branches are never relaxed with `-mattr=+relax` because MC
considers fixups where `shouldForceRelocation` returns true (which
will be the case with `+relax`) to be unresolved.
Note that binutils performs conditional branch relaxation in both cases.
This patch proposes to perform conditional branch relaxation even when
the target cannot be resolved.
Note on llvm/test/MC/RISCV/long-conditional-jump.s: I've removed the
`.p2align` because this causes alignment nops to be inserted for the
`+relax` tests. This in turn causes all the branch targets to change
compared to the non-`+relax` tests. Since `+relax` shouldn't change
these offsets, I found this confusing and hence chose to remove the
alignment.
Reviewed By: asb, MaskRay, reames
Differential Revision: https://reviews.llvm.org/D154958
Added:
Modified:
lld/test/ELF/riscv-branch.s
lld/test/ELF/riscv-undefined-weak.s
llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
llvm/test/MC/RISCV/compressed-relocations.s
llvm/test/MC/RISCV/long-conditional-jump.s
llvm/test/MC/RISCV/relocations.s
Removed:
################################################################################
diff --git a/lld/test/ELF/riscv-branch.s b/lld/test/ELF/riscv-branch.s
index b2b85fccc82d0c..dbf39dc0bb8f25 100644
--- a/lld/test/ELF/riscv-branch.s
+++ b/lld/test/ELF/riscv-branch.s
@@ -1,7 +1,7 @@
# REQUIRES: riscv
-# RUN: llvm-mc -filetype=obj -triple=riscv32-unknown-elf -mattr=-relax %s -o %t.rv32.o
-# RUN: llvm-mc -filetype=obj -triple=riscv64-unknown-elf -mattr=-relax %s -o %t.rv64.o
+# RUN: llvm-mc -filetype=obj -triple=riscv32-unknown-elf -mattr=-relax -riscv-asm-relax-branches=0 %s -o %t.rv32.o
+# RUN: llvm-mc -filetype=obj -triple=riscv64-unknown-elf -mattr=-relax -riscv-asm-relax-branches=0 %s -o %t.rv64.o
# RUN: ld.lld %t.rv32.o --defsym foo=_start+4 --defsym bar=_start -o %t.rv32
# RUN: ld.lld %t.rv64.o --defsym foo=_start+4 --defsym bar=_start -o %t.rv64
diff --git a/lld/test/ELF/riscv-undefined-weak.s b/lld/test/ELF/riscv-undefined-weak.s
index 457b7dc45ad4e6..8682fe645e9a8e 100644
--- a/lld/test/ELF/riscv-undefined-weak.s
+++ b/lld/test/ELF/riscv-undefined-weak.s
@@ -1,5 +1,5 @@
# REQUIRES: riscv
-# RUN: llvm-mc -filetype=obj -triple=riscv64 %s -o %t.o
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -riscv-asm-relax-branches=0 %s -o %t.o
# RUN: llvm-readobj -r %t.o | FileCheck --check-prefix=RELOC %s
# RUN: ld.lld -e absolute %t.o -o %t
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
index 6a46fffbb7b475..85cf21c096b7c0 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
@@ -154,12 +154,6 @@ bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCFixup &Fixup,
int64_t Offset = int64_t(Value);
unsigned Kind = Fixup.getTargetKind();
- // We only do conditional branch relaxation when the symbol is resolved.
- // For conditional branch, the immediate must be in the range
- // [-4096, 4094].
- if (Kind == RISCV::fixup_riscv_branch)
- return Resolved && !isInt<13>(Offset);
-
// Return true if the symbol is actually unresolved.
// Resolved could be always false when shouldForceRelocation return true.
// We use !WasForced to indicate that the symbol is unresolved and not forced
@@ -178,6 +172,10 @@ bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCFixup &Fixup,
// For compressed jump instructions the immediate must be
// in the range [-2048, 2046].
return Offset > 2046 || Offset < -2048;
+ case RISCV::fixup_riscv_branch:
+ // For conditional branch instructions the immediate must be
+ // in the range [-4096, 4095].
+ return !isInt<13>(Offset);
}
}
diff --git a/llvm/test/MC/RISCV/compressed-relocations.s b/llvm/test/MC/RISCV/compressed-relocations.s
index 03b9993f98612d..c7117ab702434c 100644
--- a/llvm/test/MC/RISCV/compressed-relocations.s
+++ b/llvm/test/MC/RISCV/compressed-relocations.s
@@ -17,6 +17,7 @@ c.jal foo
c.bnez a0, foo
# A compressed branch (c.bnez) to an unresolved symbol will be relaxed to a (bnez).
-# RELOC: R_RISCV_BRANCH
+# The (bnez) to an unresolved symbol will in turn be relaxed to (beqz; jal)
+# RELOC: R_RISCV_JAL
# INSTR: c.bnez a0, foo
# FIXUP: fixup A - offset: 0, value: foo, kind: fixup_riscv_rvc_branch
diff --git a/llvm/test/MC/RISCV/long-conditional-jump.s b/llvm/test/MC/RISCV/long-conditional-jump.s
index 27d3d105838d33..6c068e86ad480b 100644
--- a/llvm/test/MC/RISCV/long-conditional-jump.s
+++ b/llvm/test/MC/RISCV/long-conditional-jump.s
@@ -4,15 +4,26 @@
# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+c %s \
# RUN: | llvm-objdump -d -M no-aliases - \
# RUN: | FileCheck --check-prefix=CHECK-INST-C %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax %s \
+# RUN: | llvm-objdump -dr -M no-aliases - \
+# RUN: | FileCheck --check-prefix=CHECK-INST-RELAX %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+c,+relax %s \
+# RUN: | llvm-objdump -dr -M no-aliases - \
+# RUN: | FileCheck --check-prefix=CHECK-INST-C-RELAX %s
.text
- .p2align 3
.type test, at function
test:
# CHECK-INST: beq a0, a1, 0x8
# CHECK-INST-NEXT: jal zero, 0x1458
# CHECK-INST-C: beq a0, a1, 0x8
# CHECK-INST-C-NEXT: jal zero, 0x1458
+# CHECK-INST-RELAX: beq a0, a1, 0x8
+# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L1
+# CHECK-INST-C-RELAX: beq a0, a1, 0x8
+# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L1
bne a0, a1, .L1
.fill 1300, 4, 0
.L1:
@@ -21,6 +32,12 @@ test:
# CHECK-INST-NEXT: jal zero, 0x28b4
# CHECK-INST-C: bne a0, a1, 0x1462
# CHECK-INST-C-NEXT: jal zero, 0x28b2
+# CHECK-INST-RELAX: bne a0, a1, 0x1464
+# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L2
+# CHECK-INST-C-RELAX: bne a0, a1, 0x1462
+# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L2
beq a0, a1, .L2
.fill 1300, 4, 0
.L2:
@@ -29,6 +46,12 @@ test:
# CHECK-INST-NEXT: jal zero, 0x3d10
# CHECK-INST-C: bge a0, a1, 0x28bc
# CHECK-INST-C-NEXT: jal zero, 0x3d0c
+# CHECK-INST-RELAX: bge a0, a1, 0x28c0
+# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L3
+# CHECK-INST-C-RELAX: bge a0, a1, 0x28bc
+# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L3
blt a0, a1, .L3
.fill 1300, 4, 0
.L3:
@@ -37,6 +60,12 @@ test:
# CHECK-INST-NEXT: jal zero, 0x516c
# CHECK-INST-C: blt a0, a1, 0x3d16
# CHECK-INST-C-NEXT: jal zero, 0x5166
+# CHECK-INST-RELAX: blt a0, a1, 0x3d1c
+# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L4
+# CHECK-INST-C-RELAX: blt a0, a1, 0x3d16
+# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L4
bge a0, a1, .L4
.fill 1300, 4, 0
.L4:
@@ -45,6 +74,12 @@ test:
# CHECK-INST-NEXT: jal zero, 0x65c8
# CHECK-INST-C: bgeu a0, a1, 0x5170
# CHECK-INST-C-NEXT: jal zero, 0x65c0
+# CHECK-INST-RELAX: bgeu a0, a1, 0x5178
+# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L5
+# CHECK-INST-C-RELAX: bgeu a0, a1, 0x5170
+# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L5
bltu a0, a1, .L5
.fill 1300, 4, 0
.L5:
@@ -53,6 +88,12 @@ test:
# CHECK-INST-NEXT: jal zero, 0x7a24
# CHECK-INST-C: bltu a0, a1, 0x65ca
# CHECK-INST-C-NEXT: jal zero, 0x7a1a
+# CHECK-INST-RELAX: bltu a0, a1, 0x65d4
+# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L6
+# CHECK-INST-C-RELAX: bltu a0, a1, 0x65ca
+# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L6
bgeu a0, a1, .L6
.fill 1300, 4, 0
.L6:
@@ -61,6 +102,12 @@ test:
# CHECK-INST-NEXT: jal zero, 0x8e80
# CHECK-INST-C: c.bnez a0, 0x7a22
# CHECK-INST-C-NEXT: jal zero, 0x8e72
+# CHECK-INST-RELAX: bne a0, zero, 0x7a30
+# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L7
+# CHECK-INST-C-RELAX: c.bnez a0, 0x7a22
+# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L7
beqz a0, .L7
.fill 1300, 4, 0
.L7:
@@ -69,6 +116,12 @@ test:
# CHECK-INST-NEXT: jal zero, 0xa2dc
# CHECK-INST-C: c.bnez a0, 0x8e7a
# CHECK-INST-C-NEXT: jal zero, 0xa2ca
+# CHECK-INST-RELAX: bne zero, a0, 0x8e8c
+# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L8
+# CHECK-INST-C-RELAX: c.bnez a0, 0x8e7a
+# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L8
beq x0, a0, .L8
.fill 1300, 4, 0
.L8:
@@ -77,6 +130,12 @@ test:
# CHECK-INST-NEXT: jal zero, 0xb738
# CHECK-INST-C: c.beqz a0, 0xa2d2
# CHECK-INST-C-NEXT: jal zero, 0xb722
+# CHECK-INST-RELAX: beq a0, zero, 0xa2e8
+# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L9
+# CHECK-INST-C-RELAX: c.beqz a0, 0xa2d2
+# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L9
bnez a0, .L9
.fill 1300, 4, 0
.L9:
@@ -85,6 +144,12 @@ test:
# CHECK-INST-NEXT: jal zero, 0xcb94
# CHECK-INST-C: beq a6, zero, 0xb72c
# CHECK-INST-C-NEXT: jal zero, 0xcb7c
+# CHECK-INST-RELAX: beq a6, zero, 0xb744
+# CHECK-INST-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-RELAX-NEXT: R_RISCV_JAL .L10
+# CHECK-INST-C-RELAX: beq a6, zero, 0xb72c
+# CHECK-INST-C-RELAX-NEXT: jal zero, {{.*}}
+# CHECK-INST-C-RELAX-NEXT: R_RISCV_JAL .L10
bnez x16, .L10
.fill 1300, 4, 0
.L10:
diff --git a/llvm/test/MC/RISCV/relocations.s b/llvm/test/MC/RISCV/relocations.s
index d35bdc887e9f8e..262b3e44c6f007 100644
--- a/llvm/test/MC/RISCV/relocations.s
+++ b/llvm/test/MC/RISCV/relocations.s
@@ -171,7 +171,8 @@ jal zero, foo
# INSTR: jal zero, foo
# FIXUP: fixup A - offset: 0, value: foo, kind: fixup_riscv_jal
+# Since foo is undefined, this will be relaxed to (bltu; jal)
bgeu a0, a1, foo
-# RELOC: R_RISCV_BRANCH
+# RELOC: R_RISCV_JAL
# INSTR: bgeu a0, a1, foo
# FIXUP: fixup A - offset: 0, value: foo, kind: fixup_riscv_branch
More information about the llvm-commits
mailing list