[llvm] [RISCV] Don't delete all fixups in RISCVMCCodeEmitter::expandLongCondBr. (PR #109513)

[Craig Topper via llvm-commits]

https://github.com/topperc updated https://github.com/llvm/llvm-project/pull/109513

>From 9b05a6f097cef7acc59d9266f464a7d14142943e Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 20 Sep 2024 21:50:08 -0700
Subject: [PATCH 1/2] [RISCV} Don't delete all fixups in
 RISCVMCCodeEmitter::expandLongCondBr.

The Fixups vector passed into this function may already have fixups
in it from earlier instructions. We should not erase those. We
just want to erase fixups added by this function.

Fixes #108612.
---
 .../RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp |  7 ++++++-
 .../MC/RISCV/long-conditional-jump-crash.s    | 19 +++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 llvm/test/MC/RISCV/long-conditional-jump-crash.s

diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index 75323632dd5333..66394dc8cd138b 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -283,13 +283,18 @@ void RISCVMCCodeEmitter::expandLongCondBr(const MCInst &MI,
     Offset = 4;
   }
 
+  // Save the number fixups.
+  size_t N = Fixups.size();
+
   // Emit an unconditional jump to the destination.
   MCInst TmpInst =
       MCInstBuilder(RISCV::JAL).addReg(RISCV::X0).addOperand(SrcSymbol);
   uint32_t Binary = getBinaryCodeForInstr(TmpInst, Fixups, STI);
   support::endian::write(CB, Binary, llvm::endianness::little);
 
-  Fixups.clear();
+  // Drop any fixup added so we can add the correct one.
+  Fixups.resize(N);
+
   if (SrcSymbol.isExpr()) {
     Fixups.push_back(MCFixup::create(Offset, SrcSymbol.getExpr(),
                                      MCFixupKind(RISCV::fixup_riscv_jal),
diff --git a/llvm/test/MC/RISCV/long-conditional-jump-crash.s b/llvm/test/MC/RISCV/long-conditional-jump-crash.s
new file mode 100644
index 00000000000000..bac0036ca5568f
--- /dev/null
+++ b/llvm/test/MC/RISCV/long-conditional-jump-crash.s
@@ -0,0 +1,19 @@
+# RUN: llvm-mc %s -mc-relax-all -triple=riscv64 -filetype=obj \
+# RUN:     | llvm-objdump -d -M no-aliases - \
+# RUN:     | FileCheck --check-prefix=CHECK %s
+
+# This test previously crashed because expanding a conditional branch deleted
+# all fixups in the fragment.
+
+# CHECK:      beq     s0, zero, 0x8
+# CHECK-NEXT: jal     zero, 0x14
+# CHECK-NEXT: jal     zero, 0x14
+# CHECK-NEXT: bne     s0, zero, 0x14
+# CHECK-NEXT: jal     zero, 0x14
+
+# CHECK:      jalr    zero, 0x0(ra)
+  bnez s0, .foo
+  j    .foo
+  beqz s0, .foo
+.foo:
+  ret

>From de87c275f7d4021c307d75eec24d6c758b5c1083 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Sun, 22 Sep 2024 16:49:19 -0700
Subject: [PATCH 2/2] fixup! address review comments.

---
 .../RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp |  4 ++--
 .../MC/RISCV/long-conditional-jump-crash.s    | 19 -------------------
 llvm/test/MC/RISCV/rv64-relax-all.s           |  6 ++++++
 3 files changed, 8 insertions(+), 21 deletions(-)
 delete mode 100644 llvm/test/MC/RISCV/long-conditional-jump-crash.s

diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index 66394dc8cd138b..eb21498d15e86c 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -284,7 +284,7 @@ void RISCVMCCodeEmitter::expandLongCondBr(const MCInst &MI,
   }
 
   // Save the number fixups.
-  size_t N = Fixups.size();
+  size_t FixupStartIndex = Fixups.size();
 
   // Emit an unconditional jump to the destination.
   MCInst TmpInst =
@@ -293,7 +293,7 @@ void RISCVMCCodeEmitter::expandLongCondBr(const MCInst &MI,
   support::endian::write(CB, Binary, llvm::endianness::little);
 
   // Drop any fixup added so we can add the correct one.
-  Fixups.resize(N);
+  Fixups.resize(FixupStartIndex);
 
   if (SrcSymbol.isExpr()) {
     Fixups.push_back(MCFixup::create(Offset, SrcSymbol.getExpr(),
diff --git a/llvm/test/MC/RISCV/long-conditional-jump-crash.s b/llvm/test/MC/RISCV/long-conditional-jump-crash.s
deleted file mode 100644
index bac0036ca5568f..00000000000000
--- a/llvm/test/MC/RISCV/long-conditional-jump-crash.s
+++ /dev/null
@@ -1,19 +0,0 @@
-# RUN: llvm-mc %s -mc-relax-all -triple=riscv64 -filetype=obj \
-# RUN:     | llvm-objdump -d -M no-aliases - \
-# RUN:     | FileCheck --check-prefix=CHECK %s
-
-# This test previously crashed because expanding a conditional branch deleted
-# all fixups in the fragment.
-
-# CHECK:      beq     s0, zero, 0x8
-# CHECK-NEXT: jal     zero, 0x14
-# CHECK-NEXT: jal     zero, 0x14
-# CHECK-NEXT: bne     s0, zero, 0x14
-# CHECK-NEXT: jal     zero, 0x14
-
-# CHECK:      jalr    zero, 0x0(ra)
-  bnez s0, .foo
-  j    .foo
-  beqz s0, .foo
-.foo:
-  ret
diff --git a/llvm/test/MC/RISCV/rv64-relax-all.s b/llvm/test/MC/RISCV/rv64-relax-all.s
index 70a3f77540c997..6705d6ecfb5b62 100644
--- a/llvm/test/MC/RISCV/rv64-relax-all.s
+++ b/llvm/test/MC/RISCV/rv64-relax-all.s
@@ -14,3 +14,9 @@ c.beqz a0, NEAR
 # INSTR:           c.j    0x0 <NEAR>
 # RELAX-INSTR:     jal    zero, 0x0 <NEAR>
 c.j NEAR
+
+bnez s0, .foo
+j    .foo
+beqz s0, .foo
+.foo:
+ret