Skip to content

Commit 74c6895

Browse files
committed
[RISCV] Fix missing cross-block VSETVLI insertion
This patch fixes a codegen bug, the test for which was introduced in D112223. When merging VSETVLIInfo across blocks, if the 'exit' VSETVLIInfo produced by a block is found to be compatible with the VSETVLIInfo computed as the intersection of the 'exit' VSETVLIInfo produced by the block's predecessors, that blocks' 'exit' info is discarded and the intersected value is taken in its place. However, we have one authority on what constitutes VSETVLIInfo compatibility and we are using it in two different contexts. Compatibility is used in one context to elide VSETVLIs between straight-line vector instructions. But compatibility when evaluated between two blocks' exit infos ignores any info produced *inside* each respective block before the exit points. As such it does not guarantee that a block will not produce a VSETVLI which is incompatible with the 'previous' block. As such, we must ensure that any merging of VSETVLIInfo is performed using some notion of "strict" compatibility. I've defined this as a full vtype match, but this is perhaps too pessimistic. Given that test coverage in this regard is lacking -- the only change is in the failing test -- I think this is a good starting point. Reviewed By: craig.topper Differential Revision: https://reviews.llvm.org/D112228
1 parent e5b87fb commit 74c6895

File tree

2 files changed

+10
-8
lines changed

2 files changed

+10
-8
lines changed

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

+9-4
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ class VSETVLIInfo {
181181
// Determine whether the vector instructions requirements represented by
182182
// InstrInfo are compatible with the previous vsetvli instruction represented
183183
// by this.
184-
bool isCompatible(const VSETVLIInfo &InstrInfo) const {
184+
bool isCompatible(const VSETVLIInfo &InstrInfo, bool Strict) const {
185185
assert(isValid() && InstrInfo.isValid() &&
186186
"Can't compare invalid VSETVLIInfos");
187187
assert(!InstrInfo.SEWLMULRatioOnly &&
@@ -196,7 +196,8 @@ class VSETVLIInfo {
196196

197197
// If the instruction doesn't need an AVLReg and the SEW matches, consider
198198
// it compatible.
199-
if (InstrInfo.hasAVLReg() && InstrInfo.AVLReg == RISCV::NoRegister) {
199+
if (!Strict && InstrInfo.hasAVLReg() &&
200+
InstrInfo.AVLReg == RISCV::NoRegister) {
200201
if (SEW == InstrInfo.SEW)
201202
return true;
202203
}
@@ -209,6 +210,10 @@ class VSETVLIInfo {
209210
if (hasSameVTYPE(InstrInfo))
210211
return true;
211212

213+
// Strict matches must ensure a full VTYPE match.
214+
if (Strict)
215+
return false;
216+
212217
// If this is a mask reg operation, it only cares about VLMAX.
213218
// FIXME: Mask reg operations are probably ok if "this" VLMAX is larger
214219
// than "InstrInfo".
@@ -317,7 +322,7 @@ class VSETVLIInfo {
317322

318323
// If the change is compatible with the input, we won't create a VSETVLI
319324
// and should keep the predecessor.
320-
if (isCompatible(Other))
325+
if (isCompatible(Other, /*Strict*/ true))
321326
return *this;
322327

323328
// Otherwise just use whatever is in this block.
@@ -550,7 +555,7 @@ static VSETVLIInfo getInfoForVSETVLI(const MachineInstr &MI) {
550555

551556
bool RISCVInsertVSETVLI::needVSETVLI(const VSETVLIInfo &Require,
552557
const VSETVLIInfo &CurInfo) {
553-
if (CurInfo.isCompatible(Require))
558+
if (CurInfo.isCompatible(Require, /*Strict*/ false))
554559
return false;
555560

556561
// We didn't find a compatible value. If our AVL is a virtual register,

llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir

+1-4
Original file line numberDiff line numberDiff line change
@@ -442,10 +442,6 @@ body: |
442442
443443
...
444444
---
445-
# FIXME: We incorrectly merge info for %bb.1 into that for %bb.0, leading us to
446-
# skip a vsetvli for the PseudoVADD_VX_MF2 in %bb.3. In fact, the
447-
# PseudoVPOPC_M_B1 is given a vsetvli (e8,mf8) which, if control flow flows
448-
# through %bb.1 to %bb.3, misconfigures the PseudoVADD_VX_MF2.
449445
name: vsetvli_vpopc
450446
tracksRegLiveness: true
451447
registers:
@@ -495,6 +491,7 @@ body: |
495491
; CHECK-NEXT: {{ $}}
496492
; CHECK-NEXT: bb.3:
497493
; CHECK-NEXT: [[PHI:%[0-9]+]]:gpr = PHI [[DEF]], %bb.1, [[LWU]], %bb.2
494+
; CHECK-NEXT: dead $x0 = PseudoVSETVLIX0 killed $x0, 87, implicit-def $vl, implicit-def $vtype, implicit $vl
498495
; CHECK-NEXT: [[PseudoVADD_VX_MF2_:%[0-9]+]]:vr = nsw PseudoVADD_VX_MF2 [[PseudoVLE32_V_MF2_MASK]], [[PHI]], -1, 5, implicit $vl, implicit $vtype
499496
; CHECK-NEXT: $v0 = COPY [[PseudoVADD_VX_MF2_]]
500497
; CHECK-NEXT: PseudoRET implicit $v0

0 commit comments

Comments
 (0)