[llvm] [RISCV][VLOPT] Fix assertion failure across blocks (PR #124734)
Luke Lau via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 28 07:53:02 PST 2025
https://github.com/lukel97 updated https://github.com/llvm/llvm-project/pull/124734
>From a35e91e1a5f967f847d847638ba8758d8b0eb337 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Tue, 28 Jan 2025 19:34:28 +0800
Subject: [PATCH 1/2] [RISCV][VLOPT] Fix assertion failure across blocks
Whilst adding a cross-block test, I encountered an assertion failure in the second pass where we check the instruction popped off the worklist is a candidate.
The leaf instruction %c in this case will be added to the worklist when its VL is VLMAX, but during the first pass it will have its VL reduced to 1.
Then in the second pass when its processed via the worklist, isCandidate will no longer be true due to its VL == 1.
I think the easiest fix for this is to remove the VL > 1 check in isCandidate, so now it just checks static properties about the MCInstrDesc.
---
llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp | 10 -----
llvm/test/CodeGen/RISCV/rvv/vl-opt.mir | 46 ++++++++++++++++++++++
2 files changed, 46 insertions(+), 10 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
index e8c01e57038bfb..9bd3895489da10 100644
--- a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
+++ b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
@@ -1143,16 +1143,6 @@ bool RISCVVLOptimizer::isCandidate(const MachineInstr &MI) const {
if (MI.getNumDefs() != 1)
return false;
- unsigned VLOpNum = RISCVII::getVLOpNum(Desc);
- const MachineOperand &VLOp = MI.getOperand(VLOpNum);
-
- // If the VL is 1, then there is no need to reduce it. This is an
- // optimization, not needed to preserve correctness.
- if (VLOp.isImm() && VLOp.getImm() == 1) {
- LLVM_DEBUG(dbgs() << " Not a candidate because VL is already 1\n");
- return false;
- }
-
if (MI.mayRaiseFPException()) {
LLVM_DEBUG(dbgs() << "Not a candidate because may raise FP exception\n");
return false;
diff --git a/llvm/test/CodeGen/RISCV/rvv/vl-opt.mir b/llvm/test/CodeGen/RISCV/rvv/vl-opt.mir
index 027eb8ca3c17f0..7a28eaaaa5d8be 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vl-opt.mir
+++ b/llvm/test/CodeGen/RISCV/rvv/vl-opt.mir
@@ -149,3 +149,49 @@ body: |
; CHECK-NEXT: early-clobber %y:vrm2 = PseudoVWADD_WV_M1_TIED $noreg, %x, 1, 3 /* e8 */, 0 /* tu, mu */
%x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 0 /* tu, mu */
%y:vrm2 = PseudoVWADD_WV_M1_TIED $noreg, %x, 1, 3 /* e8 */, 0 /* tu, mu */
+...
+---
+name: crossbb
+body: |
+ ; CHECK-LABEL: name: crossbb
+ ; CHECK: bb.0:
+ ; CHECK-NEXT: successors: %bb.3(0x80000000)
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: PseudoBR %bb.3
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.1:
+ ; CHECK-NEXT: %a1:vr = PseudoVADD_VV_M1 $noreg, %c, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
+ ; CHECK-NEXT: %a2:vr = PseudoVADD_VV_M1 $noreg, %a1, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
+ ; CHECK-NEXT: $v8 = COPY %a2
+ ; CHECK-NEXT: PseudoRET
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.2:
+ ; CHECK-NEXT: %b1:vr = PseudoVADD_VV_M1 $noreg, %c, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
+ ; CHECK-NEXT: %b2:vr = PseudoVADD_VV_M1 $noreg, %b1, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
+ ; CHECK-NEXT: $v8 = COPY %b2
+ ; CHECK-NEXT: PseudoRET
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: bb.3:
+ ; CHECK-NEXT: successors: %bb.1(0x40000000), %bb.2(0x40000000)
+ ; CHECK-NEXT: liveins: $x1
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: %c:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
+ ; CHECK-NEXT: BEQ $x1, $x0, %bb.1
+ ; CHECK-NEXT: PseudoBR %bb.2
+ bb.0:
+ PseudoBR %bb.3
+ bb.1:
+ %a1:vr = PseudoVADD_VV_M1 $noreg, %c, $noreg, -1, 3 /* e8 */, 0 /* tu, mu */
+ %a2:vr = PseudoVADD_VV_M1 $noreg, %a1, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
+ $v8 = COPY %a2
+ PseudoRET
+ bb.2:
+ %b1:vr = PseudoVADD_VV_M1 $noreg, %c, $noreg, -1, 3 /* e8 */, 0 /* tu, mu */
+ %b2:vr = PseudoVADD_VV_M1 $noreg, %b1, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
+ $v8 = COPY %b2
+ PseudoRET
+ bb.3:
+ liveins: $x1
+ %c:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 0 /* tu, mu */
+ BEQ $x1, $x0, %bb.1
+ PseudoBR %bb.2
>From 4ca7c87861882d44175689fd52863db87d0a2974 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Tue, 28 Jan 2025 23:52:16 +0800
Subject: [PATCH 2/2] Keep VL = 1 check, check isCandidate after popping
instead
---
llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
index 9bd3895489da10..cb0c1b82b734d0 100644
--- a/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
+++ b/llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp
@@ -1143,6 +1143,16 @@ bool RISCVVLOptimizer::isCandidate(const MachineInstr &MI) const {
if (MI.getNumDefs() != 1)
return false;
+ unsigned VLOpNum = RISCVII::getVLOpNum(Desc);
+ const MachineOperand &VLOp = MI.getOperand(VLOpNum);
+
+ // If the VL is 1, then there is no need to reduce it. This is an
+ // optimization, not needed to preserve correctness.
+ if (VLOp.isImm() && VLOp.getImm() == 1) {
+ LLVM_DEBUG(dbgs() << " Not a candidate because VL is already 1\n");
+ return false;
+ }
+
if (MI.mayRaiseFPException()) {
LLVM_DEBUG(dbgs() << "Not a candidate because may raise FP exception\n");
return false;
@@ -1335,8 +1345,6 @@ bool RISCVVLOptimizer::runOnMachineFunction(MachineFunction &MF) {
continue;
MachineInstr *DefMI = MRI->getVRegDef(Op.getReg());
- if (!isCandidate(*DefMI))
- continue;
if (IgnoreSameBlock && DefMI->getParent() == MI.getParent())
continue;
@@ -1368,7 +1376,8 @@ bool RISCVVLOptimizer::runOnMachineFunction(MachineFunction &MF) {
while (!Worklist.empty()) {
assert(MadeChange);
MachineInstr &MI = *Worklist.pop_back_val();
- assert(isCandidate(MI));
+ if (!isCandidate(MI))
+ continue;
if (!tryReduceVL(MI))
continue;
PushOperands(MI, /*IgnoreSameBlock*/ false);
More information about the llvm-commits
mailing list