Skip to content

Commit d4cd325

Browse files
committed
[RISCV] Partially move doPeepholeMaskedRVV into RISCVFoldMasks
This change is motived by a point of confusion on llvm#71764. I hadn't fully understood why doPeepholeMaskedRVV needed to be part of the same change. As indicated in the fixme in this patch, the reason is that performCombineVMergeAndVOps doesn't know how to deal with the true side of the merge being a all-ones masked instruction. This change removes one of two calls to the routine in RISCVISELDAGToDAG, and adds a clarifying comment on the precondition for the remaining call. The post-ISEL code is tested by the cases where we can form a unmasked instruction after folding the vmerge back into true. I don't really care if we actually land this patch, or leave it roled into llvm#71764. I'm posting it mostly to clarify the confusion.
1 parent c05ab7b commit d4cd325

File tree

2 files changed

+50
-2
lines changed

2 files changed

+50
-2
lines changed

llvm/lib/Target/RISCV/RISCVFoldMasks.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
#include "RISCV.h"
2020
#include "RISCVSubtarget.h"
21+
#include "RISCVISelDAGToDAG.h"
2122
#include "llvm/CodeGen/MachineFunctionPass.h"
2223
#include "llvm/CodeGen/MachineRegisterInfo.h"
2324
#include "llvm/CodeGen/TargetInstrInfo.h"
@@ -48,6 +49,7 @@ class RISCVFoldMasks : public MachineFunctionPass {
4849
StringRef getPassName() const override { return "RISC-V Fold Masks"; }
4950

5051
private:
52+
bool convertToUnmasked(MachineInstr &MI, MachineInstr *MaskDef);
5153
bool convertVMergeToVMv(MachineInstr &MI, MachineInstr *MaskDef);
5254

5355
bool isAllOnesMask(MachineInstr *MaskDef);
@@ -132,6 +134,49 @@ bool RISCVFoldMasks::convertVMergeToVMv(MachineInstr &MI, MachineInstr *V0Def) {
132134
return true;
133135
}
134136

137+
bool RISCVFoldMasks::convertToUnmasked(MachineInstr &MI,
138+
MachineInstr *MaskDef) {
139+
const RISCV::RISCVMaskedPseudoInfo *I =
140+
RISCV::getMaskedPseudoInfo(MI.getOpcode());
141+
if (!I)
142+
return false;
143+
144+
if (!isAllOnesMask(MaskDef))
145+
return false;
146+
147+
// There are two classes of pseudos in the table - compares and
148+
// everything else. See the comment on RISCVMaskedPseudo for details.
149+
const unsigned Opc = I->UnmaskedPseudo;
150+
const MCInstrDesc &MCID = TII->get(Opc);
151+
const bool HasPolicyOp = RISCVII::hasVecPolicyOp(MCID.TSFlags);
152+
const bool HasPassthru = RISCVII::isFirstDefTiedToFirstUse(MCID);
153+
#ifndef NDEBUG
154+
const MCInstrDesc &MaskedMCID = TII->get(MI.getOpcode());
155+
assert(RISCVII::hasVecPolicyOp(MaskedMCID.TSFlags) ==
156+
RISCVII::hasVecPolicyOp(MCID.TSFlags) &&
157+
"Masked and unmasked pseudos are inconsistent");
158+
assert(HasPolicyOp == HasPassthru && "Unexpected pseudo structure");
159+
#endif
160+
161+
MI.setDesc(MCID);
162+
163+
// TODO: Increment all MaskOpIdxs in tablegen by num of explicit defs?
164+
unsigned MaskOpIdx = I->MaskOpIdx + MI.getNumExplicitDefs();
165+
MI.removeOperand(MaskOpIdx);
166+
167+
// The unmasked pseudo will no longer be constrained to the vrnov0 reg class,
168+
// so try and relax it to vr.
169+
MRI->recomputeRegClass(MI.getOperand(0).getReg());
170+
unsigned PassthruOpIdx = MI.getNumExplicitDefs();
171+
if (HasPassthru) {
172+
if (MI.getOperand(PassthruOpIdx).getReg() != RISCV::NoRegister)
173+
MRI->recomputeRegClass(MI.getOperand(PassthruOpIdx).getReg());
174+
} else
175+
MI.removeOperand(PassthruOpIdx);
176+
177+
return true;
178+
}
179+
135180
bool RISCVFoldMasks::runOnMachineFunction(MachineFunction &MF) {
136181
if (skipFunction(MF.getFunction()))
137182
return false;
@@ -159,6 +204,7 @@ bool RISCVFoldMasks::runOnMachineFunction(MachineFunction &MF) {
159204
CurrentV0Def = nullptr;
160205
for (MachineInstr &MI : MBB) {
161206
unsigned BaseOpc = RISCV::getRVVMCOpcode(MI.getOpcode());
207+
Changed |= convertToUnmasked(MI, CurrentV0Def);
162208
if (BaseOpc == RISCV::VMERGE_VVM)
163209
Changed |= convertVMergeToVMv(MI, CurrentV0Def);
164210

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,10 @@ void RISCVDAGToDAGISel::PostprocessISelDAG() {
151151
continue;
152152

153153
MadeChange |= doPeepholeSExtW(N);
154+
155+
// FIXME: This is here only because the VMerge transform doesn't
156+
// know how to handle masked true inputs. Once that has been moved
157+
// to post-ISEL, this can be deleted as well.
154158
MadeChange |= doPeepholeMaskedRVV(cast<MachineSDNode>(N));
155159
}
156160

@@ -3711,8 +3715,6 @@ bool RISCVDAGToDAGISel::performCombineVMergeAndVOps(SDNode *N) {
37113715
for (unsigned Idx = 1; Idx < True->getNumValues(); ++Idx)
37123716
ReplaceUses(True.getValue(Idx), SDValue(Result, Idx));
37133717

3714-
// Try to transform Result to unmasked intrinsic.
3715-
doPeepholeMaskedRVV(Result);
37163718
return true;
37173719
}
37183720

0 commit comments

Comments
 (0)