Skip to content

Commit 964c92d

Browse files
authored
[RISCV][InsertVSETVLI] Eliminate the AVLIsIgnored state (#94658)
As noted in one of the existing comments, the job AVLIsIgnored was filing was really more of a demanded field role. Since we recently realized we can use the values of VL on MI even in the backwards pass, let's exploit that to improve demanded fields, and delete AVLIsIgnored. Note that the test change is a real regression, but only incidental to this patch. The backwards pass doesn't have the information that the VL following a VL-preserving vtype is non-zero. This is an existing problem, this patch just adds a few more cases where we prove vl-preserving is legal.
1 parent c11677e commit 964c92d

9 files changed

+255
-221
lines changed

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp

Lines changed: 10 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,9 @@ DemandedFields getDemanded(const MachineInstr &MI, const RISCVSubtarget *ST) {
404404
if (RISCVII::hasSEWOp(TSFlags)) {
405405
Res.demandVTYPE();
406406
if (RISCVII::hasVLOp(TSFlags))
407-
Res.demandVL();
407+
if (const MachineOperand &VLOp = MI.getOperand(getVLOpNum(MI));
408+
!VLOp.isReg() || !VLOp.isUndef())
409+
Res.demandVL();
408410

409411
// Behavior is independent of mask policy.
410412
if (!RISCVII::usesMaskPolicy(TSFlags))
@@ -524,7 +526,6 @@ class VSETVLIInfo {
524526
AVLIsReg,
525527
AVLIsImm,
526528
AVLIsVLMAX,
527-
AVLIsIgnored,
528529
Unknown,
529530
} State = Uninitialized;
530531

@@ -564,12 +565,9 @@ class VSETVLIInfo {
564565

565566
void setAVLVLMAX() { State = AVLIsVLMAX; }
566567

567-
void setAVLIgnored() { State = AVLIsIgnored; }
568-
569568
bool hasAVLImm() const { return State == AVLIsImm; }
570569
bool hasAVLReg() const { return State == AVLIsReg; }
571570
bool hasAVLVLMAX() const { return State == AVLIsVLMAX; }
572-
bool hasAVLIgnored() const { return State == AVLIsIgnored; }
573571
Register getAVLReg() const {
574572
assert(hasAVLReg() && AVLRegDef.DefReg.isVirtual());
575573
return AVLRegDef.DefReg;
@@ -600,8 +598,6 @@ class VSETVLIInfo {
600598
setAVLRegDef(Info.getAVLVNInfo(), Info.getAVLReg());
601599
else if (Info.hasAVLVLMAX())
602600
setAVLVLMAX();
603-
else if (Info.hasAVLIgnored())
604-
setAVLIgnored();
605601
else {
606602
assert(Info.hasAVLImm());
607603
setAVLImm(Info.getAVLImm());
@@ -622,8 +618,6 @@ class VSETVLIInfo {
622618
}
623619
if (hasAVLVLMAX())
624620
return true;
625-
if (hasAVLIgnored())
626-
return false;
627621
return false;
628622
}
629623

@@ -645,9 +639,6 @@ class VSETVLIInfo {
645639
if (hasAVLVLMAX())
646640
return Other.hasAVLVLMAX() && hasSameVLMAX(Other);
647641

648-
if (hasAVLIgnored())
649-
return Other.hasAVLIgnored();
650-
651642
return false;
652643
}
653644

@@ -821,8 +812,6 @@ class VSETVLIInfo {
821812
OS << "AVLImm=" << (unsigned)AVLImm;
822813
if (hasAVLVLMAX())
823814
OS << "AVLVLMAX";
824-
if (hasAVLIgnored())
825-
OS << "AVLIgnored";
826815
OS << ", "
827816
<< "VLMul=" << (unsigned)VLMul << ", "
828817
<< "SEW=" << (unsigned)SEW << ", "
@@ -938,7 +927,8 @@ RISCVInsertVSETVLI::getInfoForVSETVLI(const MachineInstr &MI) const {
938927
if (AVLReg == RISCV::X0)
939928
NewInfo.setAVLVLMAX();
940929
else if (MI.getOperand(1).isUndef())
941-
NewInfo.setAVLIgnored();
930+
// Otherwise use an AVL of 1 to avoid depending on previous vl.
931+
NewInfo.setAVLImm(1);
942932
else {
943933
VNInfo *VNI = getVNInfoFromReg(AVLReg, MI, LIS);
944934
NewInfo.setAVLRegDef(VNI, AVLReg);
@@ -1014,17 +1004,17 @@ RISCVInsertVSETVLI::computeInfoForInstr(const MachineInstr &MI) const {
10141004
else
10151005
InstrInfo.setAVLImm(Imm);
10161006
} else if (VLOp.isUndef()) {
1017-
InstrInfo.setAVLIgnored();
1007+
// Otherwise use an AVL of 1 to avoid depending on previous vl.
1008+
InstrInfo.setAVLImm(1);
10181009
} else {
10191010
VNInfo *VNI = getVNInfoFromReg(VLOp.getReg(), MI, LIS);
10201011
InstrInfo.setAVLRegDef(VNI, VLOp.getReg());
10211012
}
10221013
} else {
10231014
assert(isScalarExtractInstr(MI));
1024-
// TODO: If we are more clever about x0,x0 insertion then we should be able
1025-
// to deduce that the VL is ignored based off of DemandedFields, and remove
1026-
// the AVLIsIgnored state. Then we can just use an arbitrary immediate AVL.
1027-
InstrInfo.setAVLIgnored();
1015+
// Pick a random value for state tracking purposes, will be ignored via
1016+
// the demanded fields mechanism
1017+
InstrInfo.setAVLImm(1);
10281018
}
10291019
#ifndef NDEBUG
10301020
if (std::optional<unsigned> EEW = getEEWForLoadStore(MI)) {
@@ -1104,28 +1094,6 @@ void RISCVInsertVSETVLI::insertVSETVLI(MachineBasicBlock &MBB,
11041094
return;
11051095
}
11061096

1107-
if (Info.hasAVLIgnored()) {
1108-
// We can only use x0, x0 if there's no chance of the vtype change causing
1109-
// the previous vl to become invalid.
1110-
if (PrevInfo.isValid() && !PrevInfo.isUnknown() &&
1111-
Info.hasSameVLMAX(PrevInfo)) {
1112-
auto MI = BuildMI(MBB, InsertPt, DL, TII->get(RISCV::PseudoVSETVLIX0))
1113-
.addReg(RISCV::X0, RegState::Define | RegState::Dead)
1114-
.addReg(RISCV::X0, RegState::Kill)
1115-
.addImm(Info.encodeVTYPE())
1116-
.addReg(RISCV::VL, RegState::Implicit);
1117-
LIS->InsertMachineInstrInMaps(*MI);
1118-
return;
1119-
}
1120-
// Otherwise use an AVL of 1 to avoid depending on previous vl.
1121-
auto MI = BuildMI(MBB, InsertPt, DL, TII->get(RISCV::PseudoVSETIVLI))
1122-
.addReg(RISCV::X0, RegState::Define | RegState::Dead)
1123-
.addImm(1)
1124-
.addImm(Info.encodeVTYPE());
1125-
LIS->InsertMachineInstrInMaps(*MI);
1126-
return;
1127-
}
1128-
11291097
if (Info.hasAVLVLMAX()) {
11301098
Register DestReg = MRI->createVirtualRegister(&RISCV::GPRRegClass);
11311099
auto MI = BuildMI(MBB, InsertPt, DL, TII->get(RISCV::PseudoVSETVLIX0))
@@ -1534,11 +1502,6 @@ void RISCVInsertVSETVLI::doPRE(MachineBasicBlock &MBB) {
15341502
return;
15351503
}
15361504

1537-
// If the AVL isn't used in its predecessors then bail, since we have no AVL
1538-
// to insert a vsetvli with.
1539-
if (AvailableInfo.hasAVLIgnored())
1540-
return;
1541-
15421505
// Model the effect of changing the input state of the block MBB to
15431506
// AvailableInfo. We're looking for two issues here; one legality,
15441507
// one profitability.

0 commit comments

Comments
 (0)