-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[VPlan] Match more GEP-like in m_GetElementPtr #158019
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
@llvm/pr-subscribers-vectorizers @llvm/pr-subscribers-llvm-transforms Author: Ramkumar Ramachandra (artagnon) ChangesThe m_GetElementPtr matcher is incorrect and incomplete. Fix it to match all possible GEPs to avoid misleading users. It currently just has one use, and the change is non-functional for that use. Full diff: https://github.com/llvm/llvm-project/pull/158019.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 53291a931530f..db0f6dea254e8 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -1799,6 +1799,9 @@ class LLVM_ABI_FOR_TEST VPWidenGEPRecipe : public VPRecipeWithIRFlags {
VP_CLASSOF_IMPL(VPDef::VPWidenGEPSC)
+ /// This recipe generates a GEP instruction.
+ unsigned getOpcode() const { return Instruction::GetElementPtr; }
+
/// Generate the gep nodes.
void execute(VPTransformState &State) override;
diff --git a/llvm/lib/Transforms/Vectorize/VPlanPatternMatch.h b/llvm/lib/Transforms/Vectorize/VPlanPatternMatch.h
index 109156c1469c5..14ade616b9831 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanPatternMatch.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanPatternMatch.h
@@ -252,10 +252,10 @@ struct Recipe_match {
static bool matchRecipeAndOpcode(const VPRecipeBase *R) {
auto *DefR = dyn_cast<RecipeTy>(R);
// Check for recipes that do not have opcodes.
- if constexpr (std::is_same<RecipeTy, VPScalarIVStepsRecipe>::value ||
- std::is_same<RecipeTy, VPCanonicalIVPHIRecipe>::value ||
- std::is_same<RecipeTy, VPDerivedIVRecipe>::value ||
- std::is_same<RecipeTy, VPWidenGEPRecipe>::value)
+ if constexpr (std::is_same_v<RecipeTy, VPScalarIVStepsRecipe> ||
+ std::is_same_v<RecipeTy, VPCanonicalIVPHIRecipe> ||
+ std::is_same_v<RecipeTy, VPDerivedIVRecipe> ||
+ std::is_same_v<RecipeTy, VPVectorPointerRecipe>)
return DefR;
else
return DefR && DefR->getOpcode() == Opcode;
@@ -524,15 +524,25 @@ m_SpecificCmp(CmpPredicate MatchPred, const Op0_t &Op0, const Op1_t &Op1) {
}
template <typename Op0_t, typename Op1_t>
-using GEPLikeRecipe_match =
+using GEPLikeRecipe_match = match_combine_or<
Recipe_match<std::tuple<Op0_t, Op1_t>, Instruction::GetElementPtr,
- /*Commutative*/ false, VPWidenRecipe, VPReplicateRecipe,
- VPWidenGEPRecipe, VPInstruction>;
+ /*Commutative*/ false, VPReplicateRecipe, VPWidenGEPRecipe,
+ VPVectorPointerRecipe>,
+ match_combine_or<
+ VPInstruction_match<VPInstruction::PtrAdd, Op0_t, Op1_t>,
+ VPInstruction_match<VPInstruction::WidePtrAdd, Op0_t, Op1_t>>>;
template <typename Op0_t, typename Op1_t>
inline GEPLikeRecipe_match<Op0_t, Op1_t> m_GetElementPtr(const Op0_t &Op0,
const Op1_t &Op1) {
- return GEPLikeRecipe_match<Op0_t, Op1_t>(Op0, Op1);
+ return m_CombineOr(
+ Recipe_match<std::tuple<Op0_t, Op1_t>, Instruction::GetElementPtr,
+ /*Commutative*/ false, VPReplicateRecipe, VPWidenGEPRecipe,
+ VPVectorPointerRecipe>(Op0, Op1),
+ m_CombineOr(
+ VPInstruction_match<VPInstruction::PtrAdd, Op0_t, Op1_t>(Op0, Op1),
+ VPInstruction_match<VPInstruction::WidePtrAdd, Op0_t, Op1_t>(Op0,
+ Op1)));
}
template <typename Op0_t, typename Op1_t, typename Op2_t>
|
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.
I am a bit confused by the title; does it fix an issue or is NFC? Should it say something like cover additiona GEP-like recipes in m_GetElementPtr?
Do you plan to use this in a follow-up patch? Just wondering what the best way is to test this
The m_GetElementPtr matcher is incorrect and incomplete. Fix it to match all possible GEPs to avoid misleading users. It currently just has one use, and the change is non-functional for that use.
279e279
to
e8a9716
Compare
I wasn't able to find other opportunities to use this, but wanted to fix the fact that we have a bad matcher. Added unit-test now. |
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, thanks
The m_GetElementPtr matcher is incorrect and incomplete. Fix it to match all possible GEPs to avoid misleading users. It currently just has one use, and the change is non-functional for that use.