Skip to content

Commit

Permalink
[RISCV][InsertVSETVLI] Eliminate the AVLIsIgnored state (llvm#94658)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
preames committed Jun 17, 2024
1 parent c11677e commit 964c92d
Show file tree
Hide file tree
Showing 9 changed files with 255 additions and 221 deletions.
57 changes: 10 additions & 47 deletions llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,9 @@ DemandedFields getDemanded(const MachineInstr &MI, const RISCVSubtarget *ST) {
if (RISCVII::hasSEWOp(TSFlags)) {
Res.demandVTYPE();
if (RISCVII::hasVLOp(TSFlags))
Res.demandVL();
if (const MachineOperand &VLOp = MI.getOperand(getVLOpNum(MI));
!VLOp.isReg() || !VLOp.isUndef())
Res.demandVL();

// Behavior is independent of mask policy.
if (!RISCVII::usesMaskPolicy(TSFlags))
Expand Down Expand Up @@ -524,7 +526,6 @@ class VSETVLIInfo {
AVLIsReg,
AVLIsImm,
AVLIsVLMAX,
AVLIsIgnored,
Unknown,
} State = Uninitialized;

Expand Down Expand Up @@ -564,12 +565,9 @@ class VSETVLIInfo {

void setAVLVLMAX() { State = AVLIsVLMAX; }

void setAVLIgnored() { State = AVLIsIgnored; }

bool hasAVLImm() const { return State == AVLIsImm; }
bool hasAVLReg() const { return State == AVLIsReg; }
bool hasAVLVLMAX() const { return State == AVLIsVLMAX; }
bool hasAVLIgnored() const { return State == AVLIsIgnored; }
Register getAVLReg() const {
assert(hasAVLReg() && AVLRegDef.DefReg.isVirtual());
return AVLRegDef.DefReg;
Expand Down Expand Up @@ -600,8 +598,6 @@ class VSETVLIInfo {
setAVLRegDef(Info.getAVLVNInfo(), Info.getAVLReg());
else if (Info.hasAVLVLMAX())
setAVLVLMAX();
else if (Info.hasAVLIgnored())
setAVLIgnored();
else {
assert(Info.hasAVLImm());
setAVLImm(Info.getAVLImm());
Expand All @@ -622,8 +618,6 @@ class VSETVLIInfo {
}
if (hasAVLVLMAX())
return true;
if (hasAVLIgnored())
return false;
return false;
}

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

if (hasAVLIgnored())
return Other.hasAVLIgnored();

return false;
}

Expand Down Expand Up @@ -821,8 +812,6 @@ class VSETVLIInfo {
OS << "AVLImm=" << (unsigned)AVLImm;
if (hasAVLVLMAX())
OS << "AVLVLMAX";
if (hasAVLIgnored())
OS << "AVLIgnored";
OS << ", "
<< "VLMul=" << (unsigned)VLMul << ", "
<< "SEW=" << (unsigned)SEW << ", "
Expand Down Expand Up @@ -938,7 +927,8 @@ RISCVInsertVSETVLI::getInfoForVSETVLI(const MachineInstr &MI) const {
if (AVLReg == RISCV::X0)
NewInfo.setAVLVLMAX();
else if (MI.getOperand(1).isUndef())
NewInfo.setAVLIgnored();
// Otherwise use an AVL of 1 to avoid depending on previous vl.
NewInfo.setAVLImm(1);
else {
VNInfo *VNI = getVNInfoFromReg(AVLReg, MI, LIS);
NewInfo.setAVLRegDef(VNI, AVLReg);
Expand Down Expand Up @@ -1014,17 +1004,17 @@ RISCVInsertVSETVLI::computeInfoForInstr(const MachineInstr &MI) const {
else
InstrInfo.setAVLImm(Imm);
} else if (VLOp.isUndef()) {
InstrInfo.setAVLIgnored();
// Otherwise use an AVL of 1 to avoid depending on previous vl.
InstrInfo.setAVLImm(1);
} else {
VNInfo *VNI = getVNInfoFromReg(VLOp.getReg(), MI, LIS);
InstrInfo.setAVLRegDef(VNI, VLOp.getReg());
}
} else {
assert(isScalarExtractInstr(MI));
// TODO: If we are more clever about x0,x0 insertion then we should be able
// to deduce that the VL is ignored based off of DemandedFields, and remove
// the AVLIsIgnored state. Then we can just use an arbitrary immediate AVL.
InstrInfo.setAVLIgnored();
// Pick a random value for state tracking purposes, will be ignored via
// the demanded fields mechanism
InstrInfo.setAVLImm(1);
}
#ifndef NDEBUG
if (std::optional<unsigned> EEW = getEEWForLoadStore(MI)) {
Expand Down Expand Up @@ -1104,28 +1094,6 @@ void RISCVInsertVSETVLI::insertVSETVLI(MachineBasicBlock &MBB,
return;
}

if (Info.hasAVLIgnored()) {
// We can only use x0, x0 if there's no chance of the vtype change causing
// the previous vl to become invalid.
if (PrevInfo.isValid() && !PrevInfo.isUnknown() &&
Info.hasSameVLMAX(PrevInfo)) {
auto MI = BuildMI(MBB, InsertPt, DL, TII->get(RISCV::PseudoVSETVLIX0))
.addReg(RISCV::X0, RegState::Define | RegState::Dead)
.addReg(RISCV::X0, RegState::Kill)
.addImm(Info.encodeVTYPE())
.addReg(RISCV::VL, RegState::Implicit);
LIS->InsertMachineInstrInMaps(*MI);
return;
}
// Otherwise use an AVL of 1 to avoid depending on previous vl.
auto MI = BuildMI(MBB, InsertPt, DL, TII->get(RISCV::PseudoVSETIVLI))
.addReg(RISCV::X0, RegState::Define | RegState::Dead)
.addImm(1)
.addImm(Info.encodeVTYPE());
LIS->InsertMachineInstrInMaps(*MI);
return;
}

if (Info.hasAVLVLMAX()) {
Register DestReg = MRI->createVirtualRegister(&RISCV::GPRRegClass);
auto MI = BuildMI(MBB, InsertPt, DL, TII->get(RISCV::PseudoVSETVLIX0))
Expand Down Expand Up @@ -1534,11 +1502,6 @@ void RISCVInsertVSETVLI::doPRE(MachineBasicBlock &MBB) {
return;
}

// If the AVL isn't used in its predecessors then bail, since we have no AVL
// to insert a vsetvli with.
if (AvailableInfo.hasAVLIgnored())
return;

// Model the effect of changing the input state of the block MBB to
// AvailableInfo. We're looking for two issues here; one legality,
// one profitability.
Expand Down
Loading

0 comments on commit 964c92d

Please sign in to comment.