-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[RISCV] Clear vill for whole vector register moves in vsetvli insertion #118283
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
Changes from all commits
501815a
1d86ed1
9440a86
cedea18
a0e2911
c27fba5
80256ff
a83299d
48047eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,12 @@ using namespace llvm; | |
| STATISTIC(NumInsertedVSETVL, "Number of VSETVL inst inserted"); | ||
| STATISTIC(NumCoalescedVSETVL, "Number of VSETVL inst coalesced"); | ||
|
|
||
| static cl::opt<bool> EnsureWholeVectorRegisterMoveValidVTYPE( | ||
| DEBUG_TYPE "-whole-vector-register-move-valid-vtype", cl::Hidden, | ||
| cl::desc("Insert vsetvlis before vmvNr.vs to ensure vtype is valid and " | ||
| "vill is cleared"), | ||
| cl::init(true)); | ||
|
|
||
| namespace { | ||
|
|
||
| /// Given a virtual register \p Reg, return the corresponding VNInfo for it. | ||
|
|
@@ -195,6 +201,14 @@ static bool hasUndefinedPassthru(const MachineInstr &MI) { | |
| return UseMO.getReg() == RISCV::NoRegister || UseMO.isUndef(); | ||
| } | ||
|
|
||
| /// Return true if \p MI is a copy that will be lowered to one or more vmvNr.vs. | ||
| static bool isVectorCopy(const TargetRegisterInfo *TRI, | ||
| const MachineInstr &MI) { | ||
| return MI.isCopy() && MI.getOperand(0).getReg().isPhysical() && | ||
| RISCVRegisterInfo::isRVVRegClass( | ||
| TRI->getMinimalPhysRegClass(MI.getOperand(0).getReg())); | ||
| } | ||
|
|
||
| /// Which subfields of VL or VTYPE have values we need to preserve? | ||
| struct DemandedFields { | ||
| // Some unknown property of VL is used. If demanded, must preserve entire | ||
|
|
@@ -221,10 +235,13 @@ struct DemandedFields { | |
| bool SEWLMULRatio = false; | ||
| bool TailPolicy = false; | ||
| bool MaskPolicy = false; | ||
| // If this is true, we demand that VTYPE is set to some legal state, i.e. that | ||
| // vill is unset. | ||
| bool VILL = false; | ||
|
|
||
| // Return true if any part of VTYPE was used | ||
| bool usedVTYPE() const { | ||
| return SEW || LMUL || SEWLMULRatio || TailPolicy || MaskPolicy; | ||
| return SEW || LMUL || SEWLMULRatio || TailPolicy || MaskPolicy || VILL; | ||
| } | ||
|
|
||
| // Return true if any property of VL was used | ||
|
|
@@ -239,6 +256,7 @@ struct DemandedFields { | |
| SEWLMULRatio = true; | ||
| TailPolicy = true; | ||
| MaskPolicy = true; | ||
| VILL = true; | ||
| } | ||
|
|
||
| // Mark all VL properties as demanded | ||
|
|
@@ -263,6 +281,7 @@ struct DemandedFields { | |
| SEWLMULRatio |= B.SEWLMULRatio; | ||
| TailPolicy |= B.TailPolicy; | ||
| MaskPolicy |= B.MaskPolicy; | ||
| VILL |= B.VILL; | ||
| } | ||
|
|
||
| #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) | ||
|
|
@@ -308,7 +327,8 @@ struct DemandedFields { | |
| OS << ", "; | ||
| OS << "SEWLMULRatio=" << SEWLMULRatio << ", "; | ||
| OS << "TailPolicy=" << TailPolicy << ", "; | ||
| OS << "MaskPolicy=" << MaskPolicy; | ||
| OS << "MaskPolicy=" << MaskPolicy << ", "; | ||
| OS << "VILL=" << VILL; | ||
| OS << "}"; | ||
| } | ||
| #endif | ||
|
|
@@ -503,6 +523,21 @@ DemandedFields getDemanded(const MachineInstr &MI, const RISCVSubtarget *ST) { | |
| } | ||
| } | ||
|
|
||
| // In §32.16.6, whole vector register moves have a dependency on SEW. At the | ||
| // MIR level though we don't encode the element type, and it gives the same | ||
| // result whatever the SEW may be. | ||
| // | ||
| // However it does need valid SEW, i.e. vill must be cleared. The entry to a | ||
| // function, calls and inline assembly may all set it, so make sure we clear | ||
| // it for whole register copies. Do this by leaving VILL demanded. | ||
| if (isVectorCopy(ST->getRegisterInfo(), MI)) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a cl::opt for testing? I think simple having it conditional here would effectively disable this change, and having a quick way to rule in/out this change might be useful going forward.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added it under |
||
| Res.LMUL = DemandedFields::LMULNone; | ||
| Res.SEW = DemandedFields::SEWNone; | ||
| Res.SEWLMULRatio = false; | ||
| Res.TailPolicy = false; | ||
| Res.MaskPolicy = false; | ||
| } | ||
|
|
||
| return Res; | ||
| } | ||
|
|
||
|
|
@@ -1208,6 +1243,18 @@ static VSETVLIInfo adjustIncoming(VSETVLIInfo PrevInfo, VSETVLIInfo NewInfo, | |
| // legal for MI, but may not be the state requested by MI. | ||
| void RISCVInsertVSETVLI::transferBefore(VSETVLIInfo &Info, | ||
| const MachineInstr &MI) const { | ||
| if (isVectorCopy(ST->getRegisterInfo(), MI) && | ||
| (Info.isUnknown() || !Info.isValid() || Info.hasSEWLMULRatioOnly())) { | ||
preames marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Use an arbitrary but valid AVL and VTYPE so vill will be cleared. It may | ||
| // be coalesced into another vsetvli since we won't demand any fields. | ||
| VSETVLIInfo NewInfo; // Need a new VSETVLIInfo to clear SEWLMULRatioOnly | ||
| NewInfo.setAVLImm(1); | ||
| NewInfo.setVTYPE(RISCVII::VLMUL::LMUL_1, /*sew*/ 8, /*ta*/ true, | ||
| /*ma*/ true); | ||
| Info = NewInfo; | ||
| return; | ||
| } | ||
|
|
||
| if (!RISCVII::hasSEWOp(MI.getDesc().TSFlags)) | ||
| return; | ||
|
|
||
|
|
@@ -1296,7 +1343,8 @@ bool RISCVInsertVSETVLI::computeVLVTYPEChanges(const MachineBasicBlock &MBB, | |
| for (const MachineInstr &MI : MBB) { | ||
| transferBefore(Info, MI); | ||
|
|
||
| if (isVectorConfigInstr(MI) || RISCVII::hasSEWOp(MI.getDesc().TSFlags)) | ||
| if (isVectorConfigInstr(MI) || RISCVII::hasSEWOp(MI.getDesc().TSFlags) || | ||
| isVectorCopy(ST->getRegisterInfo(), MI)) | ||
| HadVectorOp = true; | ||
|
|
||
| transferAfter(Info, MI); | ||
|
|
@@ -1426,6 +1474,16 @@ void RISCVInsertVSETVLI::emitVSETVLIs(MachineBasicBlock &MBB) { | |
| PrefixTransparent = false; | ||
| } | ||
|
|
||
| if (EnsureWholeVectorRegisterMoveValidVTYPE && | ||
| isVectorCopy(ST->getRegisterInfo(), MI)) { | ||
| if (!PrevInfo.isCompatible(DemandedFields::all(), CurInfo, LIS)) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a followup, we can reduce the number of vsetvli's emitted by integrating this with the needVSETVLIPHI logic below. Definitely non blocking. |
||
| insertVSETVLI(MBB, MI, MI.getDebugLoc(), CurInfo, PrevInfo); | ||
wangpc-pp marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| PrefixTransparent = false; | ||
| } | ||
| MI.addOperand(MachineOperand::CreateReg(RISCV::VTYPE, /*isDef*/ false, | ||
| /*isImp*/ true)); | ||
| } | ||
|
|
||
| uint64_t TSFlags = MI.getDesc().TSFlags; | ||
| if (RISCVII::hasSEWOp(TSFlags)) { | ||
| if (!PrevInfo.isCompatible(DemandedFields::all(), CurInfo, LIS)) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.