-
Notifications
You must be signed in to change notification settings - Fork 13.3k
[LV][EVL] Support cast instruction with EVL-vectorization #108351
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 10 commits
28e4d5c
4d3be15
3f4e0e5
cc04719
363d625
0e45dc4
1d8f27c
5bd40b9
118f55a
6b8fb80
1e54505
b7bc8a8
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 |
---|---|---|
|
@@ -964,7 +964,8 @@ void VPWidenIntrinsicRecipe::execute(VPTransformState &State) { | |
Module *M = State.Builder.GetInsertBlock()->getModule(); | ||
Function *VectorF = | ||
Intrinsic::getOrInsertDeclaration(M, VectorIntrinsicID, TysForDecl); | ||
assert(VectorF && "Can't retrieve vector intrinsic."); | ||
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. Is this change still need? 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. Probably sufficient to keep the existing message |
||
assert(VectorF && | ||
"Can't retrieve vector intrinsic or vector-predication intrinsics."); | ||
|
||
auto *CI = cast_or_null<CallInst>(getUnderlyingValue()); | ||
SmallVector<OperandBundleDef, 1> OpBundles; | ||
|
@@ -973,11 +974,13 @@ void VPWidenIntrinsicRecipe::execute(VPTransformState &State) { | |
|
||
CallInst *V = State.Builder.CreateCall(VectorF, Args, OpBundles); | ||
|
||
// FIXME: vp.cast and vp.select dont pass the underlying instruction into the | ||
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. This comment is confusing here I think, it should be at the place where it is not passed, as this needs to be fixed where the recipe is constructed, right? 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. Removed. I will try to add Flags during the recipe construction with new patch |
||
// recipe, which set the flags and metadata be needed. | ||
setFlags(V); | ||
|
||
if (!V->getType()->isVoidTy()) | ||
State.set(this, V); | ||
State.addMetadata(V, CI); | ||
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. Is this change still need? 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. The following patch will be used, I can remove it first. Pls give LGTM :) |
||
State.addMetadata(V, dyn_cast_or_null<Instruction>(getUnderlyingValue())); | ||
} | ||
|
||
InstructionCost VPWidenIntrinsicRecipe::computeCost(ElementCount VF, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1485,8 +1485,8 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) { | |
auto *CI = cast<CallInst>(CInst->getUnderlyingInstr()); | ||
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. The TypeSwitch here has grown quite big, it would be good to outline it to a separate function to reduce the nesting level and make things slightly easier to read 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. (would be good as. a follow up) 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. OK~ |
||
Intrinsic::ID VPID = VPIntrinsic::getForIntrinsic( | ||
CI->getCalledFunction()->getIntrinsicID()); | ||
if (VPID == Intrinsic::not_intrinsic) | ||
return nullptr; | ||
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. Good change, but unrelated. 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. Yeah probably worth splitting off 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. done |
||
assert(VPID != Intrinsic::not_intrinsic && | ||
"Expected VP Instrinsic"); | ||
|
||
SmallVector<VPValue *> Ops(CInst->operands()); | ||
assert(VPIntrinsic::getMaskParamPos(VPID) && | ||
|
@@ -1495,11 +1495,32 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) { | |
VPValue *Mask = Plan.getOrAddLiveIn(ConstantInt::getTrue( | ||
IntegerType::getInt1Ty(CI->getContext()))); | ||
Ops.push_back(Mask); | ||
; | ||
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. redundant 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. removed |
||
Ops.push_back(&EVL); | ||
return new VPWidenIntrinsicRecipe( | ||
*CI, VPID, Ops, TypeInfo.inferScalarType(CInst), | ||
CInst->getDebugLoc()); | ||
}) | ||
.Case<VPWidenCastRecipe>( | ||
[&](VPWidenCastRecipe *CInst) -> VPRecipeBase * { | ||
auto *CI = dyn_cast<CastInst>(CInst->getUnderlyingInstr()); | ||
Intrinsic::ID VPID = | ||
VPIntrinsic::getForOpcode(CI->getOpcode()); | ||
assert(VPID != Intrinsic::not_intrinsic && | ||
"Expected vp.casts Instrinsic"); | ||
|
||
SmallVector<VPValue *> Ops(CInst->operands()); | ||
assert(VPIntrinsic::getMaskParamPos(VPID) && | ||
VPIntrinsic::getVectorLengthParamPos(VPID) && | ||
"Expected VP intrinsic"); | ||
VPValue *Mask = Plan.getOrAddLiveIn(ConstantInt::getTrue( | ||
IntegerType::getInt1Ty(CI->getContext()))); | ||
Ops.push_back(Mask); | ||
Ops.push_back(&EVL); | ||
return new VPWidenIntrinsicRecipe( | ||
VPID, Ops, TypeInfo.inferScalarType(CInst), | ||
CInst->getDebugLoc()); | ||
}) | ||
.Case<VPWidenSelectRecipe>([&](VPWidenSelectRecipe *Sel) { | ||
SmallVector<VPValue *> Ops(Sel->operands()); | ||
Ops.push_back(&EVL); | ||
|
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.
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.
done