-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[RISCV] Move AVL coalescing logic upwards into computeInfoForInstr. NFC #73909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There is an optimisation in transferBefore where if a VSETVLIInfo uses the AVL of a defining vsetvli, it uses that vsetvli's AVL provided VLMAX is the same. This patch moves it out of transferBefore and up into computeInfoForInstr to show how it isn't affected by the other optimisations in transferBefore, and to simplify the control flow by removing an early return. This should make llvm#72352 easier to reason about.
@llvm/pr-subscribers-backend-risc-v Author: Luke Lau (lukel97) ChangesThere is an optimisation in transferBefore where if a VSETVLIInfo uses the AVL This patch moves it out of transferBefore and up into computeInfoForInstr to This should make #72352 easier to reason about. Full diff: https://github.com/llvm/llvm-project/pull/73909.diff 1 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index 3bb648359e39dd6..a174d762bf8cc8e 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -781,6 +781,25 @@ char RISCVInsertVSETVLI::ID = 0;
INITIALIZE_PASS(RISCVInsertVSETVLI, DEBUG_TYPE, RISCV_INSERT_VSETVLI_NAME,
false, false)
+// Return a VSETVLIInfo representing the changes made by this VSETVLI or
+// VSETIVLI instruction.
+static VSETVLIInfo getInfoForVSETVLI(const MachineInstr &MI) {
+ VSETVLIInfo NewInfo;
+ if (MI.getOpcode() == RISCV::PseudoVSETIVLI) {
+ NewInfo.setAVLImm(MI.getOperand(1).getImm());
+ } else {
+ assert(MI.getOpcode() == RISCV::PseudoVSETVLI ||
+ MI.getOpcode() == RISCV::PseudoVSETVLIX0);
+ Register AVLReg = MI.getOperand(1).getReg();
+ assert((AVLReg != RISCV::X0 || MI.getOperand(0).getReg() != RISCV::X0) &&
+ "Can't handle X0, X0 vsetvli yet");
+ NewInfo.setAVLReg(AVLReg);
+ }
+ NewInfo.setVTYPE(MI.getOperand(2).getImm());
+
+ return NewInfo;
+}
+
static VSETVLIInfo computeInfoForInstr(const MachineInstr &MI, uint64_t TSFlags,
const MachineRegisterInfo *MRI) {
VSETVLIInfo InstrInfo;
@@ -841,6 +860,21 @@ static VSETVLIInfo computeInfoForInstr(const MachineInstr &MI, uint64_t TSFlags,
#endif
InstrInfo.setVTYPE(VLMul, SEW, TailAgnostic, MaskAgnostic);
+ // If AVL is defined by a vsetvli with the same VLMAX, we can replace the
+ // AVL operand with the AVL of the defining vsetvli. We avoid general
+ // register AVLs to avoid extending live ranges without being sure we can
+ // kill the original source reg entirely.
+ if (InstrInfo.hasAVLReg() && InstrInfo.getAVLReg().isVirtual()) {
+ MachineInstr *DefMI = MRI->getVRegDef(InstrInfo.getAVLReg());
+ if (DefMI && isVectorConfigInstr(*DefMI)) {
+ VSETVLIInfo DefInstrInfo = getInfoForVSETVLI(*DefMI);
+ if (DefInstrInfo.hasSameVLMAX(InstrInfo) &&
+ (DefInstrInfo.hasAVLImm() || DefInstrInfo.getAVLReg() == RISCV::X0)) {
+ InstrInfo.setAVL(DefInstrInfo);
+ }
+ }
+ }
+
return InstrInfo;
}
@@ -851,25 +885,6 @@ void RISCVInsertVSETVLI::insertVSETVLI(MachineBasicBlock &MBB, MachineInstr &MI,
insertVSETVLI(MBB, MachineBasicBlock::iterator(&MI), DL, Info, PrevInfo);
}
-// Return a VSETVLIInfo representing the changes made by this VSETVLI or
-// VSETIVLI instruction.
-static VSETVLIInfo getInfoForVSETVLI(const MachineInstr &MI) {
- VSETVLIInfo NewInfo;
- if (MI.getOpcode() == RISCV::PseudoVSETIVLI) {
- NewInfo.setAVLImm(MI.getOperand(1).getImm());
- } else {
- assert(MI.getOpcode() == RISCV::PseudoVSETVLI ||
- MI.getOpcode() == RISCV::PseudoVSETVLIX0);
- Register AVLReg = MI.getOperand(1).getReg();
- assert((AVLReg != RISCV::X0 || MI.getOperand(0).getReg() != RISCV::X0) &&
- "Can't handle X0, X0 vsetvli yet");
- NewInfo.setAVLReg(AVLReg);
- }
- NewInfo.setVTYPE(MI.getOperand(2).getImm());
-
- return NewInfo;
-}
-
void RISCVInsertVSETVLI::insertVSETVLI(MachineBasicBlock &MBB,
MachineBasicBlock::iterator InsertPt, DebugLoc DL,
const VSETVLIInfo &Info, const VSETVLIInfo &PrevInfo) {
@@ -1065,27 +1080,8 @@ void RISCVInsertVSETVLI::transferBefore(VSETVLIInfo &Info,
// to prevent extending live range of an avl register operand.
// TODO: We can probably relax this for immediates.
if (Demanded.VLZeroness && !Demanded.VLAny && PrevInfo.isValid() &&
- PrevInfo.hasEquallyZeroAVL(Info, *MRI) && Info.hasSameVLMAX(PrevInfo)) {
+ PrevInfo.hasEquallyZeroAVL(Info, *MRI) && Info.hasSameVLMAX(PrevInfo))
Info.setAVL(PrevInfo);
- return;
- }
-
- // If AVL is defined by a vsetvli with the same VLMAX, we can
- // replace the AVL operand with the AVL of the defining vsetvli.
- // We avoid general register AVLs to avoid extending live ranges
- // without being sure we can kill the original source reg entirely.
- if (!Info.hasAVLReg() || !Info.getAVLReg().isVirtual())
- return;
- MachineInstr *DefMI = MRI->getVRegDef(Info.getAVLReg());
- if (!DefMI || !isVectorConfigInstr(*DefMI))
- return;
-
- VSETVLIInfo DefInfo = getInfoForVSETVLI(*DefMI);
- if (DefInfo.hasSameVLMAX(Info) &&
- (DefInfo.hasAVLImm() || DefInfo.getAVLReg() == RISCV::X0)) {
- Info.setAVL(DefInfo);
- return;
- }
}
// Given a state with which we evaluated MI (see transferBefore above for why
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There is an optimisation in transferBefore where if a VSETVLIInfo uses the AVL
of a defining vsetvli, it uses that vsetvli's AVL provided VLMAX is the same.
This patch moves it out of transferBefore and up into computeInfoForInstr to
show how it isn't affected by the other optimisations in transferBefore, and to
simplify the control flow by removing an early return.
This should make #72352 easier to reason about.