[SLP] NFC. Use InstructionsState::valid if users just want to know whether VL has same opcode. #120217
+67
−62
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -821,21 +821,28 @@ class InstructionsState { | |||||
|
|
||||||
| /// The main/alternate opcodes for the list of instructions. | ||||||
| unsigned getOpcode() const { | ||||||
| assert(valid() && "InstructionsState is invalid."); | ||||||
| return getMainOp()->getOpcode(); | ||||||
| } | ||||||
|
|
||||||
| unsigned getAltOpcode() const { | ||||||
| assert(valid() && "InstructionsState is invalid."); | ||||||
| return getAltOp()->getOpcode(); | ||||||
| } | ||||||
|
|
||||||
| /// Some of the instructions in the list have alternate opcodes. | ||||||
| bool isAltShuffle() const { return getMainOp() != getAltOp(); } | ||||||
|
|
||||||
| bool isOpcodeOrAlt(Instruction *I) const { | ||||||
| unsigned CheckedOpcode = I->getOpcode(); | ||||||
| return getOpcode() == CheckedOpcode || getAltOpcode() == CheckedOpcode; | ||||||
| } | ||||||
|
|
||||||
| /// Checks if the current state is valid, i.e. has non-null MainOp | ||||||
| bool valid() const { return getMainOp() && getAltOp(); } | ||||||
|
|
||||||
| explicit operator bool() const { return valid(); } | ||||||
|
|
||||||
| InstructionsState() = delete; | ||||||
| InstructionsState(Instruction *MainOp, Instruction *AltOp) | ||||||
| : MainOp(MainOp), AltOp(AltOp) {} | ||||||
|
|
@@ -868,8 +875,8 @@ static bool areCompatibleCmpOps(Value *BaseOp0, Value *BaseOp1, Value *Op0, | |||||
| (!isa<Instruction>(BaseOp0) && !isa<Instruction>(Op0) && | ||||||
| !isa<Instruction>(BaseOp1) && !isa<Instruction>(Op1)) || | ||||||
| BaseOp0 == Op0 || BaseOp1 == Op1 || | ||||||
| getSameOpcode({BaseOp0, Op0}, TLI) || | ||||||
| getSameOpcode({BaseOp1, Op1}, TLI); | ||||||
| } | ||||||
|
|
||||||
| /// \returns true if a compare instruction \p CI has similar "look" and | ||||||
|
|
@@ -1845,7 +1852,7 @@ class BoUpSLP { | |||||
| InstructionsState S = getSameOpcode(Ops, TLI); | ||||||
| // Note: Only consider instructions with <= 2 operands to avoid | ||||||
| // complexity explosion. | ||||||
| if (S && | ||||||
| (S.getMainOp()->getNumOperands() <= 2 || !MainAltOps.empty() || | ||||||
| !S.isAltShuffle()) && | ||||||
| all_of(Ops, [&S](Value *V) { | ||||||
|
|
@@ -2380,7 +2387,7 @@ class BoUpSLP { | |||||
| // Use Boyer-Moore majority voting for finding the majority opcode and | ||||||
| // the number of times it occurs. | ||||||
| if (auto *I = dyn_cast<Instruction>(OpData.V)) { | ||||||
| if (!OpcodeI || !getSameOpcode({OpcodeI, I}, TLI) || | ||||||
| I->getParent() != Parent) { | ||||||
| if (NumOpsWithSameOpcodeParent == 0) { | ||||||
| NumOpsWithSameOpcodeParent = 1; | ||||||
|
|
@@ -2499,8 +2506,7 @@ class BoUpSLP { | |||||
| // 2.1. If we have only 2 lanes, need to check that value in the | ||||||
| // next lane does not build same opcode sequence. | ||||||
| (Lns == 2 && | ||||||
| !getSameOpcode({Op, getValue((OpI + 1) % OpE, Ln)}, TLI) && | ||||||
| isa<Constant>(Data.V)))) || | ||||||
| // 3. The operand in the current lane is loop invariant (can be | ||||||
| // hoisted out) and another operand is also a loop invariant | ||||||
|
|
@@ -2509,7 +2515,7 @@ class BoUpSLP { | |||||
| // FIXME: need to teach the cost model about this case for better | ||||||
| // estimation. | ||||||
| (IsInvariant && !isa<Constant>(Data.V) && | ||||||
| !getSameOpcode({Op, Data.V}, TLI) && | ||||||
| L->isLoopInvariant(Data.V))) { | ||||||
| FoundCandidate = true; | ||||||
| Data.IsUsed = Data.V == Op; | ||||||
|
|
@@ -2539,7 +2545,7 @@ class BoUpSLP { | |||||
| return true; | ||||||
| Value *OpILn = getValue(OpI, Ln); | ||||||
| return (L && L->isLoopInvariant(OpILn)) || | ||||||
| (getSameOpcode({Op, OpILn}, TLI) && | ||||||
| allSameBlock({Op, OpILn})); | ||||||
| })) | ||||||
| return true; | ||||||
|
|
@@ -2696,7 +2702,7 @@ class BoUpSLP { | |||||
| OperandData &AltOp = getData(OpIdx, Lane); | ||||||
| InstructionsState OpS = | ||||||
| getSameOpcode({MainAltOps[OpIdx].front(), AltOp.V}, TLI); | ||||||
| if (OpS && OpS.isAltShuffle()) | ||||||
| MainAltOps[OpIdx].push_back(AltOp.V); | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -3591,7 +3597,7 @@ class BoUpSLP { | |||||
| "Need to vectorize gather entry?"); | ||||||
| // Gathered loads still gathered? Do not create entry, use the original one. | ||||||
| if (GatheredLoadsEntriesFirst.has_value() && | ||||||
| EntryState == TreeEntry::NeedToGather && S && | ||||||
| S.getOpcode() == Instruction::Load && UserTreeIdx.EdgeIdx == UINT_MAX && | ||||||
| !UserTreeIdx.UserTE) | ||||||
| return nullptr; | ||||||
|
|
@@ -4765,8 +4771,7 @@ static bool arePointersCompatible(Value *Ptr1, Value *Ptr2, | |||||
| (!GEP2 || isConstant(GEP2->getOperand(1)))) || | ||||||
| !CompareOpcodes || | ||||||
| (GEP1 && GEP2 && | ||||||
| getSameOpcode({GEP1->getOperand(1), GEP2->getOperand(1)}, TLI))); | ||||||
| } | ||||||
|
|
||||||
| /// Calculates minimal alignment as a common alignment. | ||||||
|
|
@@ -7488,7 +7493,7 @@ bool BoUpSLP::areAltOperandsProfitable(const InstructionsState &S, | |||||
| [&](ArrayRef<Value *> Op) { | ||||||
| if (allConstant(Op) || | ||||||
| (!isSplat(Op) && allSameBlock(Op) && allSameType(Op) && | ||||||
| getSameOpcode(Op, *TLI))) | ||||||
| return false; | ||||||
| DenseMap<Value *, unsigned> Uniques; | ||||||
| for (Value *V : Op) { | ||||||
|
|
@@ -8059,15 +8064,14 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth, | |||||
| // Don't go into catchswitch blocks, which can happen with PHIs. | ||||||
| // Such blocks can only have PHIs and the catchswitch. There is no | ||||||
| // place to insert a shuffle if we need to, so just avoid that issue. | ||||||
| if (S && isa<CatchSwitchInst>(S.getMainOp()->getParent()->getTerminator())) { | ||||||
| LLVM_DEBUG(dbgs() << "SLP: bundle in catchswitch block.\n"); | ||||||
| newTreeEntry(VL, std::nullopt /*not vectorized*/, S, UserTreeIdx); | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| // Check if this is a duplicate of another entry. | ||||||
| if (S) { | ||||||
| if (TreeEntry *E = getTreeEntry(S.getMainOp())) { | ||||||
| LLVM_DEBUG(dbgs() << "SLP: \tChecking bundle: " << *S.getMainOp() | ||||||
| << ".\n"); | ||||||
|
|
@@ -8128,13 +8132,12 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth, | |||||
| // a load), in which case peek through to include it in the tree, without | ||||||
| // ballooning over-budget. | ||||||
| if (Depth >= RecursionMaxDepth && | ||||||
| !(S && !S.isAltShuffle() && VL.size() >= 4 && | ||||||
| (match(S.getMainOp(), m_Load(m_Value())) || | ||||||
| all_of(VL, [&S](const Value *I) { | ||||||
| return match(I, | ||||||
| m_OneUse(m_ZExtOrSExt(m_OneUse(m_Load(m_Value()))))) && | ||||||
| cast<Instruction>(I)->getOpcode() == S.getOpcode(); | ||||||
| })))) { | ||||||
| LLVM_DEBUG(dbgs() << "SLP: Gathering due to max recursion depth.\n"); | ||||||
| if (TryToFindDuplicates(S)) | ||||||
|
|
@@ -8144,9 +8147,8 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth, | |||||
| } | ||||||
|
|
||||||
| // Don't handle scalable vectors | ||||||
| if (auto *EE = dyn_cast_if_present<ExtractElementInst>(S.getMainOp()); | ||||||
|
||||||
| if (auto *EE = dyn_cast_if_present<ExtractElementInst>(S.getMainOp()); | |
| if (auto *EE = dyn_cast<ExtractElementInst>(S.getMainOp()); |
Contributor
Author
There was a problem hiding this comment.
Cannot. S may not be valid.
Member
There was a problem hiding this comment.
Then you cannot use getMainOp
Contributor
Author
There was a problem hiding this comment.
- The origin code check MainOp opcode (
S.getOpcode() == Instruction::ExtractElement) and usecastlater. I think usedyn_cast_if_presentis better.
if (S.getOpcode() == Instruction::ExtractElement &&
isa<ScalableVectorType>(
cast<ExtractElementInst>(S.getMainOp())->getVectorOperandType())) {
- There is a case which uses
isa_and_present. Follow the same idea, I usedyn_cast_if_presenthere.
bool AreAllSameInsts = AreAllSameBlock || AreScatterAllGEPSameBlock;
if (!AreAllSameInsts || (!S && allConstant(VL)) || isSplat(VL) ||
(isa_and_present<InsertElementInst, ExtractValueInst, ExtractElementInst>(
S.getMainOp()) &&
!all_of(VL, isVectorLikeInstWithConstOps)) ||
NotProfitableForVectorization(VL)) {
Member
There was a problem hiding this comment.
- I.e. the original code check if S is valid, has the opcode ExtractElement and then checks the vector type. getMainOp will crash, if S is invalid. If S is valid, getMainOp cannot return nullptr. dyn_cast_if_present should not be used here, just a dyn_cast should be enough. getMainOp just cannot return nullptr.
- It should be fixed, need to replace by
S and isa<....>(S.getMainOp()). It worked before, currently it will crash, if S is invalid
Contributor
Author
There was a problem hiding this comment.
Currently some code use isa some code use getOpcode. I think we should either
- use isa, dyn_cast, isa_and_present and dyn_cast_if_present for getMainOp
- use getOpcode and check whether the opcode is what we want
to make the code consistency.
However, isa (and others) can check isa_and_present<InsertElementInst, ExtractValueInst, ExtractElementInst> with less code. Use getOpcode will write a lot of code in this case.
Member
There was a problem hiding this comment.
It won't work, it will crash the compiler, _and_present are not required
Contributor
Author
There was a problem hiding this comment.
It can work. See c9ccb55.
isa_and_present<> = S.valid() && isa<>
Member
There was a problem hiding this comment.
It should not work. getMainOp() and getAltOp() should crash if requested from invalid state
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.