Skip to content

Commit ff271d0

Browse files
authored
[RISCV][VLOPT] Fix assertion failure across blocks (#124734)
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. This fixes it by moving the VL == 1 check to tryReduceVL, keeping it alongside the other VL check for bailing out early as an optimisation.
1 parent 6092740 commit ff271d0

File tree

2 files changed

+56
-13
lines changed

2 files changed

+56
-13
lines changed

llvm/lib/Target/RISCV/RISCVVLOptimizer.cpp

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,16 +1143,6 @@ bool RISCVVLOptimizer::isCandidate(const MachineInstr &MI) const {
11431143
if (MI.getNumDefs() != 1)
11441144
return false;
11451145

1146-
unsigned VLOpNum = RISCVII::getVLOpNum(Desc);
1147-
const MachineOperand &VLOp = MI.getOperand(VLOpNum);
1148-
1149-
// If the VL is 1, then there is no need to reduce it. This is an
1150-
// optimization, not needed to preserve correctness.
1151-
if (VLOp.isImm() && VLOp.getImm() == 1) {
1152-
LLVM_DEBUG(dbgs() << " Not a candidate because VL is already 1\n");
1153-
return false;
1154-
}
1155-
11561146
if (MI.mayRaiseFPException()) {
11571147
LLVM_DEBUG(dbgs() << "Not a candidate because may raise FP exception\n");
11581148
return false;
@@ -1285,16 +1275,23 @@ std::optional<MachineOperand> RISCVVLOptimizer::checkUsers(MachineInstr &MI) {
12851275
bool RISCVVLOptimizer::tryReduceVL(MachineInstr &MI) {
12861276
LLVM_DEBUG(dbgs() << "Trying to reduce VL for " << MI << "\n");
12871277

1278+
unsigned VLOpNum = RISCVII::getVLOpNum(MI.getDesc());
1279+
MachineOperand &VLOp = MI.getOperand(VLOpNum);
1280+
1281+
// If the VL is 1, then there is no need to reduce it. This is an
1282+
// optimization, not needed to preserve correctness.
1283+
if (VLOp.isImm() && VLOp.getImm() == 1) {
1284+
LLVM_DEBUG(dbgs() << " Abort due to VL == 1, no point in reducing.\n");
1285+
return false;
1286+
}
1287+
12881288
auto CommonVL = checkUsers(MI);
12891289
if (!CommonVL)
12901290
return false;
12911291

12921292
assert((CommonVL->isImm() || CommonVL->getReg().isVirtual()) &&
12931293
"Expected VL to be an Imm or virtual Reg");
12941294

1295-
unsigned VLOpNum = RISCVII::getVLOpNum(MI.getDesc());
1296-
MachineOperand &VLOp = MI.getOperand(VLOpNum);
1297-
12981295
if (!RISCV::isVLKnownLE(*CommonVL, VLOp)) {
12991296
LLVM_DEBUG(dbgs() << " Abort due to CommonVL not <= VLOp.\n");
13001297
return false;

llvm/test/CodeGen/RISCV/rvv/vl-opt.mir

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,3 +149,49 @@ body: |
149149
; CHECK-NEXT: early-clobber %y:vrm2 = PseudoVWADD_WV_M1_TIED $noreg, %x, 1, 3 /* e8 */, 0 /* tu, mu */
150150
%x:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 0 /* tu, mu */
151151
%y:vrm2 = PseudoVWADD_WV_M1_TIED $noreg, %x, 1, 3 /* e8 */, 0 /* tu, mu */
152+
...
153+
---
154+
name: crossbb
155+
body: |
156+
; CHECK-LABEL: name: crossbb
157+
; CHECK: bb.0:
158+
; CHECK-NEXT: successors: %bb.3(0x80000000)
159+
; CHECK-NEXT: {{ $}}
160+
; CHECK-NEXT: PseudoBR %bb.3
161+
; CHECK-NEXT: {{ $}}
162+
; CHECK-NEXT: bb.1:
163+
; CHECK-NEXT: %a1:vr = PseudoVADD_VV_M1 $noreg, %c, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
164+
; CHECK-NEXT: %a2:vr = PseudoVADD_VV_M1 $noreg, %a1, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
165+
; CHECK-NEXT: $v8 = COPY %a2
166+
; CHECK-NEXT: PseudoRET
167+
; CHECK-NEXT: {{ $}}
168+
; CHECK-NEXT: bb.2:
169+
; CHECK-NEXT: %b1:vr = PseudoVADD_VV_M1 $noreg, %c, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
170+
; CHECK-NEXT: %b2:vr = PseudoVADD_VV_M1 $noreg, %b1, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
171+
; CHECK-NEXT: $v8 = COPY %b2
172+
; CHECK-NEXT: PseudoRET
173+
; CHECK-NEXT: {{ $}}
174+
; CHECK-NEXT: bb.3:
175+
; CHECK-NEXT: successors: %bb.1(0x40000000), %bb.2(0x40000000)
176+
; CHECK-NEXT: liveins: $x1
177+
; CHECK-NEXT: {{ $}}
178+
; CHECK-NEXT: %c:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
179+
; CHECK-NEXT: BEQ $x1, $x0, %bb.1
180+
; CHECK-NEXT: PseudoBR %bb.2
181+
bb.0:
182+
PseudoBR %bb.3
183+
bb.1:
184+
%a1:vr = PseudoVADD_VV_M1 $noreg, %c, $noreg, -1, 3 /* e8 */, 0 /* tu, mu */
185+
%a2:vr = PseudoVADD_VV_M1 $noreg, %a1, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
186+
$v8 = COPY %a2
187+
PseudoRET
188+
bb.2:
189+
%b1:vr = PseudoVADD_VV_M1 $noreg, %c, $noreg, -1, 3 /* e8 */, 0 /* tu, mu */
190+
%b2:vr = PseudoVADD_VV_M1 $noreg, %b1, $noreg, 1, 3 /* e8 */, 0 /* tu, mu */
191+
$v8 = COPY %b2
192+
PseudoRET
193+
bb.3:
194+
liveins: $x1
195+
%c:vr = PseudoVADD_VV_M1 $noreg, $noreg, $noreg, -1, 3 /* e8 */, 0 /* tu, mu */
196+
BEQ $x1, $x0, %bb.1
197+
PseudoBR %bb.2

0 commit comments

Comments
 (0)