[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