Skip to content

[VPlan] Use parameter packs to avoid unary/binary/ternary matchers. NFC #152272

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.

Sign up for GitHub

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

Merged
lukel97 merged 6 commits into from
Aug 14, 2025
Merged

[VPlan] Use parameter packs to avoid unary/binary/ternary matchers. NFC #152272

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
836f0f4
[VPlan] Use parameter packs for m_VPInstruction. NFC
lukel97 Aug 6, 2025
5171b12
clang-format
lukel97 Aug 6, 2025
7c86025
Remove unary/binary/ternary recipe matchers
lukel97 Aug 6, 2025
9d4ab2b
Add AllRecipes_commutative_match, remove commutative template argument
lukel97 Aug 6, 2025
361b191
Add comment labels for commutative, demorgan assert
lukel97 Aug 8, 2025
6e789a9
Merge branch 'main' into loop-vectorize/pattern-match-parameter-pack
lukel97 Aug 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
207 changes: 64 additions & 143 deletions llvm/lib/Transforms/Vectorize/VPlanPatternMatch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,15 +200,11 @@ template <typename Ops_t, unsigned Opcode, bool Commutative,
struct Recipe_match {
Ops_t Ops;

Recipe_match() : Ops() {
static_assert(std::tuple_size<Ops_t>::value == 0 &&
"constructor can only be used with zero operands");
}
Recipe_match(Ops_t Ops) : Ops(Ops) {}
template <typename A_t, typename B_t>
Recipe_match(A_t A, B_t B) : Ops({A, B}) {
static_assert(std::tuple_size<Ops_t>::value == 2 &&
"constructor can only be used for binary matcher");
template <typename... OpTy> Recipe_match(OpTy... Ops) : Ops(Ops...) {
static_assert(std::tuple_size<Ops_t>::value == sizeof...(Ops) &&
"number of operands in constructor doesn't match Ops_t");
Comment on lines +203 to +205
Copy link
Contributor

@fhahn fhahn Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change needed? It seems orthogonal to removing the various matchers, at least initially?

Copy link
Contributor Author

@lukel97 lukel97 Aug 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping the old constructors around causes ambiguous constructor overloads, and fails to compile without. Also some places seem to construct Recipe_match with the {} syntax which invokes the Recipe_match(Ops_t Ops) constructor, so I had to remove that too.

static_assert((!Commutative || std::tuple_size<Ops_t>::value == 2) &&
"only binary ops can be commutative");
}

bool match(const VPValue *V) const {
Expand Down Expand Up @@ -254,7 +250,6 @@ struct Recipe_match {
// 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, VPWidenSelectRecipe>::value ||
Copy link
Contributor Author

@lukel97 lukel97 Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was removed because VPWidenSelectRecipe was added to AllRecipe_match (to remove TernaryRecipe_match), and we don't want unary or binary matchers to unconditionally match on VPWidenSelectRecipe.

VPWidenSelectRecipe defines getOpcode() to be Instruction::Select so just use that instead.

std::is_same<RecipeTy, VPDerivedIVRecipe>::value ||
std::is_same<RecipeTy, VPWidenGEPRecipe>::value)
return DefR;
Expand All @@ -270,195 +265,128 @@ struct Recipe_match {
}
};

template <unsigned Opcode, typename... RecipeTys>
using ZeroOpRecipe_match =
Recipe_match<std::tuple<>, Opcode, false, RecipeTys...>;

template <typename Op0_t, unsigned Opcode, typename... RecipeTys>
using UnaryRecipe_match =
Recipe_match<std::tuple<Op0_t>, Opcode, false, RecipeTys...>;

template <typename Op0_t, unsigned Opcode>
using UnaryVPInstruction_match =
UnaryRecipe_match<Op0_t, Opcode, VPInstruction>;
template <unsigned Opcode, typename... OpTys>
using AllRecipe_match =
Copy link
Contributor

@david-arm david-arm Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a thought, but is it worth making this consistent with AllRecipe_commutative_match by renaming it to AllRecipe_noncommutative_match? I realise the naming scheme was already like that before your patch though. At the moment it sounds a bit like it's a superset that includes commutative matches too.

Copy link
Contributor Author

@lukel97 lukel97 Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not opposed to this, but I guess I was trying to make it consistent with the names of the actual matchers themselves, i.e m_foo for non-commutative and m_c_foo for commutative.

Would renaming the commutative version to AllRecipe_c_match be more consistent?

Copy link
Contributor

@fhahn fhahn Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably best to keep this in-line with IR PatternMatch, for which the default is also non-commutative w/o special naming I think.

Recipe_match<std::tuple<OpTys...>, Opcode, /*Commutative*/ false,
VPWidenRecipe, VPReplicateRecipe, VPWidenCastRecipe,
VPInstruction, VPWidenSelectRecipe>;

template <unsigned Opcode>
using ZeroOpVPInstruction_match = ZeroOpRecipe_match<Opcode, VPInstruction>;
template <unsigned Opcode, typename... OpTys>
using AllRecipe_commutative_match =
Recipe_match<std::tuple<OpTys...>, Opcode, /*Commutative*/ true,
VPWidenRecipe, VPReplicateRecipe, VPInstruction>;

template <typename Op0_t, unsigned Opcode>
using AllUnaryRecipe_match =
UnaryRecipe_match<Op0_t, Opcode, VPWidenRecipe, VPReplicateRecipe,
VPWidenCastRecipe, VPInstruction>;
template <unsigned Opcode, typename... OpTys>
using VPInstruction_match = Recipe_match<std::tuple<OpTys...>, Opcode,
Copy link
Contributor

@david-arm david-arm Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not need both commutative and non-commutative variants? We use VPInstruction types for normal IR opcodes too, such as add, mul, etc. which are commutative.

Copy link
Contributor Author

@lukel97 lukel97 Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we could add m_c_VPInstruction at some point, but it didn't exist before this patch. Just doing a quick check though I can't really find any prospective users around. I think most cases that would want something like this use m_c_Binary which matches other non-VPInstruction recipes too.

/*Commutative*/ false, VPInstruction>;

template <typename Op0_t, typename Op1_t, unsigned Opcode, bool Commutative,
typename... RecipeTys>
using BinaryRecipe_match =
Recipe_match<std::tuple<Op0_t, Op1_t>, Opcode, Commutative, RecipeTys...>;

template <typename Op0_t, typename Op1_t, unsigned Opcode>
using BinaryVPInstruction_match =
BinaryRecipe_match<Op0_t, Op1_t, Opcode, /*Commutative*/ false,
VPInstruction>;

template <typename Op0_t, typename Op1_t, typename Op2_t, unsigned Opcode,
bool Commutative, typename... RecipeTys>
using TernaryRecipe_match = Recipe_match<std::tuple<Op0_t, Op1_t, Op2_t>,
Opcode, Commutative, RecipeTys...>;

template <typename Op0_t, typename Op1_t, typename Op2_t, unsigned Opcode>
using TernaryVPInstruction_match =
TernaryRecipe_match<Op0_t, Op1_t, Op2_t, Opcode, /*Commutative*/ false,
VPInstruction>;

template <typename Op0_t, typename Op1_t, unsigned Opcode,
bool Commutative = false>
using AllBinaryRecipe_match =
BinaryRecipe_match<Op0_t, Op1_t, Opcode, Commutative, VPWidenRecipe,
VPReplicateRecipe, VPWidenCastRecipe, VPInstruction>;
template <unsigned Opcode, typename... OpTys>
inline VPInstruction_match<Opcode, OpTys...>
m_VPInstruction(const OpTys &...Ops) {
return VPInstruction_match<Opcode, OpTys...>(Ops...);
}

/// BuildVector is matches only its opcode, w/o matching its operands as the
/// number of operands is not fixed.
inline ZeroOpVPInstruction_match<VPInstruction::BuildVector> m_BuildVector() {
return ZeroOpVPInstruction_match<VPInstruction::BuildVector>();
}

template <unsigned Opcode, typename Op0_t>
inline UnaryVPInstruction_match<Op0_t, Opcode>
m_VPInstruction(const Op0_t &Op0) {
return UnaryVPInstruction_match<Op0_t, Opcode>(Op0);
}

template <unsigned Opcode, typename Op0_t, typename Op1_t>
inline BinaryVPInstruction_match<Op0_t, Op1_t, Opcode>
m_VPInstruction(const Op0_t &Op0, const Op1_t &Op1) {
return BinaryVPInstruction_match<Op0_t, Op1_t, Opcode>(Op0, Op1);
inline VPInstruction_match<VPInstruction::BuildVector> m_BuildVector() {
return m_VPInstruction<VPInstruction::BuildVector>();
}

template <unsigned Opcode, typename Op0_t, typename Op1_t, typename Op2_t>
inline TernaryVPInstruction_match<Op0_t, Op1_t, Op2_t, Opcode>
m_VPInstruction(const Op0_t &Op0, const Op1_t &Op1, const Op2_t &Op2) {
return TernaryVPInstruction_match<Op0_t, Op1_t, Op2_t, Opcode>(
{Op0, Op1, Op2});
}

template <typename Op0_t, typename Op1_t, typename Op2_t, typename Op3_t,
unsigned Opcode, bool Commutative, typename... RecipeTys>
using Recipe4Op_match = Recipe_match<std::tuple<Op0_t, Op1_t, Op2_t, Op3_t>,
Opcode, Commutative, RecipeTys...>;

template <typename Op0_t, typename Op1_t, typename Op2_t, typename Op3_t,
unsigned Opcode>
using VPInstruction4Op_match =
Recipe4Op_match<Op0_t, Op1_t, Op2_t, Op3_t, Opcode, /*Commutative*/ false,
VPInstruction>;

template <unsigned Opcode, typename Op0_t, typename Op1_t, typename Op2_t,
typename Op3_t>
inline VPInstruction4Op_match<Op0_t, Op1_t, Op2_t, Op3_t, Opcode>
m_VPInstruction(const Op0_t &Op0, const Op1_t &Op1, const Op2_t &Op2,
const Op3_t &Op3) {
return VPInstruction4Op_match<Op0_t, Op1_t, Op2_t, Op3_t, Opcode>(
{Op0, Op1, Op2, Op3});
}
template <typename Op0_t>
inline UnaryVPInstruction_match<Op0_t, Instruction::Freeze>
inline VPInstruction_match<Instruction::Freeze, Op0_t>
m_Freeze(const Op0_t &Op0) {
return m_VPInstruction<Instruction::Freeze>(Op0);
}

template <typename Op0_t>
inline UnaryVPInstruction_match<Op0_t, VPInstruction::BranchOnCond>
inline VPInstruction_match<VPInstruction::BranchOnCond, Op0_t>
m_BranchOnCond(const Op0_t &Op0) {
return m_VPInstruction<VPInstruction::BranchOnCond>(Op0);
}

template <typename Op0_t>
inline UnaryVPInstruction_match<Op0_t, VPInstruction::Broadcast>
inline VPInstruction_match<VPInstruction::Broadcast, Op0_t>
m_Broadcast(const Op0_t &Op0) {
return m_VPInstruction<VPInstruction::Broadcast>(Op0);
}

template <typename Op0_t, typename Op1_t>
inline BinaryVPInstruction_match<Op0_t, Op1_t, VPInstruction::ActiveLaneMask>
inline VPInstruction_match<VPInstruction::ActiveLaneMask, Op0_t, Op1_t>
m_ActiveLaneMask(const Op0_t &Op0, const Op1_t &Op1) {
return m_VPInstruction<VPInstruction::ActiveLaneMask>(Op0, Op1);
}

template <typename Op0_t, typename Op1_t>
inline BinaryVPInstruction_match<Op0_t, Op1_t, VPInstruction::BranchOnCount>
inline VPInstruction_match<VPInstruction::BranchOnCount, Op0_t, Op1_t>
m_BranchOnCount(const Op0_t &Op0, const Op1_t &Op1) {
return m_VPInstruction<VPInstruction::BranchOnCount>(Op0, Op1);
}

template <unsigned Opcode, typename Op0_t>
inline AllUnaryRecipe_match<Op0_t, Opcode> m_Unary(const Op0_t &Op0) {
return AllUnaryRecipe_match<Op0_t, Opcode>(Op0);
inline AllRecipe_match<Opcode, Op0_t> m_Unary(const Op0_t &Op0) {
return AllRecipe_match<Opcode, Op0_t>(Op0);
}

template <typename Op0_t>
inline AllUnaryRecipe_match<Op0_t, Instruction::Trunc>
m_Trunc(const Op0_t &Op0) {
inline AllRecipe_match<Instruction::Trunc, Op0_t> m_Trunc(const Op0_t &Op0) {
return m_Unary<Instruction::Trunc, Op0_t>(Op0);
}

template <typename Op0_t>
inline AllUnaryRecipe_match<Op0_t, Instruction::ZExt> m_ZExt(const Op0_t &Op0) {
inline AllRecipe_match<Instruction::ZExt, Op0_t> m_ZExt(const Op0_t &Op0) {
return m_Unary<Instruction::ZExt, Op0_t>(Op0);
}

template <typename Op0_t>
inline AllUnaryRecipe_match<Op0_t, Instruction::SExt> m_SExt(const Op0_t &Op0) {
inline AllRecipe_match<Instruction::SExt, Op0_t> m_SExt(const Op0_t &Op0) {
return m_Unary<Instruction::SExt, Op0_t>(Op0);
}

template <typename Op0_t>
inline match_combine_or<AllUnaryRecipe_match<Op0_t, Instruction::ZExt>,
AllUnaryRecipe_match<Op0_t, Instruction::SExt>>
inline match_combine_or<AllRecipe_match<Instruction::ZExt, Op0_t>,
AllRecipe_match<Instruction::SExt, Op0_t>>
m_ZExtOrSExt(const Op0_t &Op0) {
return m_CombineOr(m_ZExt(Op0), m_SExt(Op0));
}

template <unsigned Opcode, typename Op0_t, typename Op1_t,
bool Commutative = false>
inline AllBinaryRecipe_match<Op0_t, Op1_t, Opcode, Commutative>
m_Binary(const Op0_t &Op0, const Op1_t &Op1) {
return AllBinaryRecipe_match<Op0_t, Op1_t, Opcode, Commutative>(Op0, Op1);
template <unsigned Opcode, typename Op0_t, typename Op1_t>
inline AllRecipe_match<Opcode, Op0_t, Op1_t> m_Binary(const Op0_t &Op0,
const Op1_t &Op1) {
return AllRecipe_match<Opcode, Op0_t, Op1_t>(Op0, Op1);
}

template <unsigned Opcode, typename Op0_t, typename Op1_t>
inline AllBinaryRecipe_match<Op0_t, Op1_t, Opcode, true>
inline AllRecipe_commutative_match<Opcode, Op0_t, Op1_t>
m_c_Binary(const Op0_t &Op0, const Op1_t &Op1) {
return AllBinaryRecipe_match<Op0_t, Op1_t, Opcode, true>(Op0, Op1);
return AllRecipe_commutative_match<Opcode, Op0_t, Op1_t>(Op0, Op1);
}

template <typename Op0_t, typename Op1_t>
inline AllBinaryRecipe_match<Op0_t, Op1_t, Instruction::Mul>
m_Mul(const Op0_t &Op0, const Op1_t &Op1) {
inline AllRecipe_match<Instruction::Mul, Op0_t, Op1_t> m_Mul(const Op0_t &Op0,
const Op1_t &Op1) {
return m_Binary<Instruction::Mul, Op0_t, Op1_t>(Op0, Op1);
}

template <typename Op0_t, typename Op1_t>
inline AllBinaryRecipe_match<Op0_t, Op1_t, Instruction::Mul,
/* Commutative =*/true>
inline AllRecipe_commutative_match<Instruction::Mul, Op0_t, Op1_t>
m_c_Mul(const Op0_t &Op0, const Op1_t &Op1) {
return m_Binary<Instruction::Mul, Op0_t, Op1_t, true>(Op0, Op1);
return m_c_Binary<Instruction::Mul, Op0_t, Op1_t>(Op0, Op1);
}

/// Match a binary OR operation. Note that while conceptually the operands can
/// be matched commutatively, \p Commutative defaults to false in line with the
/// IR-based pattern matching infrastructure. Use m_c_BinaryOr for a commutative
/// version of the matcher.
template <typename Op0_t, typename Op1_t, bool Commutative = false>
inline AllBinaryRecipe_match<Op0_t, Op1_t, Instruction::Or, Commutative>
template <typename Op0_t, typename Op1_t>
inline AllRecipe_match<Instruction::Or, Op0_t, Op1_t>
m_BinaryOr(const Op0_t &Op0, const Op1_t &Op1) {
return m_Binary<Instruction::Or, Op0_t, Op1_t, Commutative>(Op0, Op1);
return m_Binary<Instruction::Or, Op0_t, Op1_t>(Op0, Op1);
}

template <typename Op0_t, typename Op1_t>
inline AllBinaryRecipe_match<Op0_t, Op1_t, Instruction::Or,
/*Commutative*/ true>
inline AllRecipe_commutative_match<Instruction::Or, Op0_t, Op1_t>
m_c_BinaryOr(const Op0_t &Op0, const Op1_t &Op1) {
return m_BinaryOr<Op0_t, Op1_t, /*Commutative*/ true>(Op0, Op1);
return m_c_Binary<Instruction::Or, Op0_t, Op1_t>(Op0, Op1);
}

/// ICmp_match is a variant of BinaryRecipe_match that also binds the comparison
Expand Down Expand Up @@ -523,58 +451,51 @@ m_SpecificICmp(CmpPredicate MatchPred, const Op0_t &Op0, const Op1_t &Op1) {

template <typename Op0_t, typename Op1_t>
using GEPLikeRecipe_match =
BinaryRecipe_match<Op0_t, Op1_t, Instruction::GetElementPtr, false,
VPWidenRecipe, VPReplicateRecipe, VPWidenGEPRecipe,
VPInstruction>;
Recipe_match<std::tuple<Op0_t, Op1_t>, Instruction::GetElementPtr,
/*Commutative*/ false, VPWidenRecipe, VPReplicateRecipe,
VPWidenGEPRecipe, VPInstruction>;

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);
}

template <typename Op0_t, typename Op1_t, typename Op2_t, unsigned Opcode>
using AllTernaryRecipe_match =
Recipe_match<std::tuple<Op0_t, Op1_t, Op2_t>, Opcode, false,
VPReplicateRecipe, VPInstruction, VPWidenSelectRecipe>;

template <typename Op0_t, typename Op1_t, typename Op2_t>
inline AllTernaryRecipe_match<Op0_t, Op1_t, Op2_t, Instruction::Select>
inline AllRecipe_match<Instruction::Select, Op0_t, Op1_t, Op2_t>
m_Select(const Op0_t &Op0, const Op1_t &Op1, const Op2_t &Op2) {
return AllTernaryRecipe_match<Op0_t, Op1_t, Op2_t, Instruction::Select>(
return AllRecipe_match<Instruction::Select, Op0_t, Op1_t, Op2_t>(
{Op0, Op1, Op2});
}

template <typename Op0_t>
inline match_combine_or<UnaryVPInstruction_match<Op0_t, VPInstruction::Not>,
AllBinaryRecipe_match<int_pred_ty<is_all_ones>, Op0_t,
Instruction::Xor, true>>
inline match_combine_or<VPInstruction_match<VPInstruction::Not, Op0_t>,
AllRecipe_commutative_match<
Instruction::Xor, int_pred_ty<is_all_ones>, Op0_t>>
m_Not(const Op0_t &Op0) {
return m_CombineOr(m_VPInstruction<VPInstruction::Not>(Op0),
m_c_Binary<Instruction::Xor>(m_AllOnes(), Op0));
}

template <typename Op0_t, typename Op1_t>
inline match_combine_or<
BinaryVPInstruction_match<Op0_t, Op1_t, VPInstruction::LogicalAnd>,
AllTernaryRecipe_match<Op0_t, Op1_t, specific_intval<1>,
Instruction::Select>>
VPInstruction_match<VPInstruction::LogicalAnd, Op0_t, Op1_t>,
AllRecipe_match<Instruction::Select, Op0_t, Op1_t, specific_intval<1>>>
m_LogicalAnd(const Op0_t &Op0, const Op1_t &Op1) {
return m_CombineOr(
m_VPInstruction<VPInstruction::LogicalAnd, Op0_t, Op1_t>(Op0, Op1),
m_Select(Op0, Op1, m_False()));
}

template <typename Op0_t, typename Op1_t>
inline AllTernaryRecipe_match<Op0_t, specific_intval<1>, Op1_t,
Instruction::Select>
inline AllRecipe_match<Instruction::Select, Op0_t, specific_intval<1>, Op1_t>
m_LogicalOr(const Op0_t &Op0, const Op1_t &Op1) {
return m_Select(Op0, m_True(), Op1);
}

template <typename Op0_t, typename Op1_t, typename Op2_t>
using VPScalarIVSteps_match =
TernaryRecipe_match<Op0_t, Op1_t, Op2_t, 0, false, VPScalarIVStepsRecipe>;
using VPScalarIVSteps_match = Recipe_match<std::tuple<Op0_t, Op1_t, Op2_t>, 0,
false, VPScalarIVStepsRecipe>;
Comment on lines +497 to +498
Copy link
Contributor

@artagnon artagnon Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
using VPScalarIVSteps_match = Recipe_match<std::tuple<Op0_t, Op1_t, Op2_t>, 0,
false, VPScalarIVStepsRecipe>;
using VPScalarIVSteps_match = Recipe_match<std::tuple<Op0_t, Op1_t, Op2_t>, /*Opcode*/ 0,
/*Commutative*/ false, VPScalarIVStepsRecipe>;


template <typename Op0_t, typename Op1_t, typename Op2_t>
inline VPScalarIVSteps_match<Op0_t, Op1_t, Op2_t>
Expand Down
Loading