-
Notifications
You must be signed in to change notification settings - Fork 13.7k
[VectorCombine] Fold ZExt/SExt (Shuffle (ZExt/SExt %src)) to ZExt/SExt (Shuffle %src). #141109
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -128,6 +128,7 @@ class VectorCombine { | |
bool foldShuffleOfShuffles(Instruction &I); | ||
bool foldShuffleOfIntrinsics(Instruction &I); | ||
bool foldShuffleToIdentity(Instruction &I); | ||
bool foldShuffleExt(Instruction &I); | ||
bool foldShuffleFromReductions(Instruction &I); | ||
bool foldCastFromReductions(Instruction &I); | ||
bool foldSelectShuffle(Instruction &I, bool FromReduction = false); | ||
|
@@ -2791,6 +2792,60 @@ bool VectorCombine::foldShuffleToIdentity(Instruction &I) { | |
return true; | ||
} | ||
|
||
bool VectorCombine::foldShuffleExt(Instruction &I) { | ||
// Try to fold vector zero- and sign-extends split across multiple operations | ||
// into a single extend. | ||
|
||
// Check if we have ZEXT/SEXT (SHUFFLE (ZEXT/SEXT %src), _, identity-mask), | ||
// with an identity mask extracting the first sub-vector. | ||
Value *Src; | ||
ArrayRef<int> Mask; | ||
if (!match(&I, m_OneUse(m_Shuffle(m_OneUse(m_ZExtOrSExt(m_Value(Src))), | ||
m_Value(), m_Mask(Mask)))) || | ||
!cast<ShuffleVectorInst>(&I)->isIdentityWithExtract()) | ||
return false; | ||
auto *InnerExt = cast<Instruction>(I.getOperand(0)); | ||
auto *OuterExt = cast<Instruction>(*I.user_begin()); | ||
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 OuterExt cast ever fail? 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 don't think so, it is a user of an instruction, which in turn should require it itself to be an instruction. |
||
if (!isa<SExtInst, ZExtInst>(OuterExt)) | ||
return false; | ||
|
||
// If the inner extend is a sign extend and the outer one isnt (i.e. a | ||
// zero-extend), don't fold. If the first one is zero-extend, it doesn't | ||
// matter if the second one is a sign- or zero-extend. | ||
if (isa<SExtInst>(InnerExt) && !isa<SExtInst>(OuterExt)) | ||
return false; | ||
|
||
auto *DstTy = cast<FixedVectorType>(OuterExt->getType()); | ||
auto *SrcTy = | ||
FixedVectorType::get(InnerExt->getOperand(0)->getType()->getScalarType(), | ||
DstTy->getNumElements()); | ||
|
||
// Don't perform the fold if the cost of the new extend is worse than the cost | ||
// of the 2 original extends. | ||
InstructionCost OriginalCost = | ||
TTI.getCastInstrCost(InnerExt->getOpcode(), SrcTy, InnerExt->getType(), | ||
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. It doesn't look like you have the right params for costing the original outer param - the inner params are repeated twice. |
||
TTI::CastContextHint::None) + | ||
TTI.getCastInstrCost(InnerExt->getOpcode(), SrcTy, InnerExt->getType(), | ||
TTI::CastContextHint::None); | ||
InstructionCost NewCost = TTI.getCastInstrCost( | ||
InnerExt->getOpcode(), SrcTy, DstTy, TTI::CastContextHint::None); | ||
if (NewCost > OriginalCost) | ||
return false; | ||
|
||
// Convert to a shuffle of the input feeding a single wide extend. | ||
Builder.SetInsertPoint(*OuterExt->getInsertionPointAfterDef()); | ||
auto *NewIns = | ||
Builder.CreateShuffleVector(Src, PoisonValue::get(Src->getType()), Mask); | ||
auto *NewExt = | ||
isa<ZExtInst>(InnerExt) | ||
? Builder.CreateZExt(NewIns, DstTy, "vec.ext", InnerExt->hasNonNeg()) | ||
: Builder.CreateSExt(NewIns, DstTy, "vec.ext"); | ||
OuterExt->replaceAllUsesWith(NewExt); | ||
replaceValue(*OuterExt, *NewExt); | ||
Worklist.pushValue(NewExt); | ||
return true; | ||
} | ||
|
||
/// Given a commutative reduction, the order of the input lanes does not alter | ||
/// the results. We can use this to remove certain shuffles feeding the | ||
/// reduction, removing the need to shuffle at all. | ||
|
@@ -3565,6 +3620,7 @@ bool VectorCombine::run() { | |
break; | ||
case Instruction::ShuffleVector: | ||
MadeChange |= widenSubvectorLoad(I); | ||
MadeChange |= foldShuffleExt(I); | ||
break; | ||
default: | ||
break; | ||
|
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.
Code structure wise, I think you're rooting this from the wrong node. I think this should start from the outer extend, and then match the whole pattern.