Skip to content

Commit 15184d8

Browse files
committed
Address some review comments (still more to come)
- Update comments - Move one use check - Rename helper to make it more clear it's only being used to check ActiveElementsAffectResult - Only change passthru if needed
1 parent d0e913f commit 15184d8

File tree

1 file changed

+29
-28
lines changed

1 file changed

+29
-28
lines changed

llvm/lib/Target/RISCV/RISCVVectorPeephole.cpp

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -358,51 +358,46 @@ static bool isSafeToMove(const MachineInstr &From, const MachineInstr &To) {
358358
break;
359359
}
360360
}
361-
return From.isSafeToMove(nullptr, SawStore);
361+
return From.isSafeToMove(SawStore);
362362
}
363363

364-
static const RISCV::RISCVMaskedPseudoInfo *
365-
lookupMaskedPseudoInfo(const MachineInstr &MI) {
364+
static std::optional<bool>
365+
lookupActiveElementsAffectsResult(const MachineInstr &MI) {
366366
const RISCV::RISCVMaskedPseudoInfo *Info =
367367
RISCV::lookupMaskedIntrinsicByUnmasked(MI.getOpcode());
368368
if (!Info)
369369
Info = RISCV::getMaskedPseudoInfo(MI.getOpcode());
370-
return Info;
370+
if (!Info)
371+
return std::nullopt;
372+
return Info->ActiveElementsAffectResult;
371373
}
372374

373-
/// If a PseudoVMV_V_V is the only user of it's input, fold its passthru and VL
375+
/// If a PseudoVMV_V_V is the only user of its input, fold its passthru and VL
374376
/// into it.
375377
///
376-
/// %x = PseudoVADD_V_V_M1 %passthru, %a, %b, %vl, sew, policy
377-
/// %y = PseudoVMV_V_V_M1 %passthru, %x, %vl, sew, policy
378+
/// %x = PseudoVADD_V_V_M1 %passthru, %a, %b, %vl1, sew, policy
379+
/// %y = PseudoVMV_V_V_M1 %passthru, %x, %vl2, sew, policy
378380
///
379381
/// ->
380382
///
381-
/// %y = PseudoVADD_V_V_M1 %passthru, %a, %b, %vl, sew, policy
383+
/// %y = PseudoVADD_V_V_M1 %passthru, %a, %b, min(vl1, vl2), sew, policy
382384
bool RISCVVectorPeephole::foldVMV_V_V(MachineInstr &MI) {
383385
if (RISCV::getRVVMCOpcode(MI.getOpcode()) != RISCV::VMV_V_V)
384386
return false;
385387

386388
MachineOperand &Passthru = MI.getOperand(1);
387-
MachineInstr *Src = MRI->getVRegDef(MI.getOperand(2).getReg());
388389

389390
if (!MRI->hasOneUse(MI.getOperand(2).getReg()))
390391
return false;
391392

393+
MachineInstr *Src = MRI->getVRegDef(MI.getOperand(2).getReg());
392394
if (!Src || Src->hasUnmodeledSideEffects() ||
393-
Src->getParent() != MI.getParent())
394-
return false;
395-
396-
// Src needs to be a pseudo that's opted into this transform.
397-
const RISCV::RISCVMaskedPseudoInfo *Info = lookupMaskedPseudoInfo(*Src);
398-
if (!Info)
395+
Src->getParent() != MI.getParent() || Src->getNumDefs() != 1 ||
396+
!RISCVII::isFirstDefTiedToFirstUse(Src->getDesc()) ||
397+
!RISCVII::hasVLOp(Src->getDesc().TSFlags) ||
398+
!RISCVII::hasVecPolicyOp(Src->getDesc().TSFlags))
399399
return false;
400400

401-
assert(Src->getNumDefs() == 1 &&
402-
RISCVII::isFirstDefTiedToFirstUse(Src->getDesc()) &&
403-
RISCVII::hasVLOp(Src->getDesc().TSFlags) &&
404-
RISCVII::hasVecPolicyOp(Src->getDesc().TSFlags));
405-
406401
// Src needs to have the same passthru as VMV_V_V
407402
if (Src->getOperand(1).getReg() != RISCV::NoRegister &&
408403
Src->getOperand(1).getReg() != Passthru.getReg())
@@ -428,21 +423,27 @@ bool RISCVVectorPeephole::foldVMV_V_V(MachineInstr &MI) {
428423
bool VLChanged = !MinVL->isIdenticalTo(SrcVL);
429424
bool RaisesFPExceptions = MI.getDesc().mayRaiseFPException() &&
430425
!MI.getFlag(MachineInstr::MIFlag::NoFPExcept);
431-
if (VLChanged && (Info->ActiveElementsAffectResult || RaisesFPExceptions))
426+
auto ActiveElementsAffectResult = lookupActiveElementsAffectsResult(*Src);
427+
if (!ActiveElementsAffectResult)
428+
return false;
429+
if (VLChanged && (*ActiveElementsAffectResult || RaisesFPExceptions))
432430
return false;
433431

434432
if (!isSafeToMove(*Src, MI))
435433
return false;
436434

437-
// Move Src down to MI, then replace all uses of MI with it.
435+
// Move Src down to MI so it can access its passthru/VL, then replace all uses
436+
// of MI with it.
438437
Src->moveBefore(&MI);
439438

440-
Src->getOperand(1).setReg(Passthru.getReg());
441-
// If Src is masked then its passthru needs to be in VRNoV0.
442-
if (Passthru.getReg() != RISCV::NoRegister)
443-
MRI->constrainRegClass(Passthru.getReg(),
444-
TII->getRegClass(Src->getDesc(), 1, TRI,
445-
*Src->getParent()->getParent()));
439+
if (Src->getOperand(1).getReg() != Passthru.getReg()) {
440+
Src->getOperand(1).setReg(Passthru.getReg());
441+
// If Src is masked then its passthru needs to be in VRNoV0.
442+
if (Passthru.getReg() != RISCV::NoRegister)
443+
MRI->constrainRegClass(Passthru.getReg(),
444+
TII->getRegClass(Src->getDesc(), 1, TRI,
445+
*Src->getParent()->getParent()));
446+
}
446447

447448
if (MinVL->isImm())
448449
SrcVL.ChangeToImmediate(MinVL->getImm());

0 commit comments

Comments
 (0)