[llvm] Add SD matchers and unit test coverage for ISD::VECTOR_SHUFFLE (PR #119592)
Min-Yih Hsu via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 26 10:22:02 PST 2024
================
@@ -548,49 +548,39 @@ struct BinaryOpc_match {
};
/// Matching while capturing mask
-template <typename T0, typename T1> struct SDShuffle_match {
+template <typename T0, typename T1, typename T2> struct SDShuffle_match {
T0 Op1;
T1 Op2;
+ T2 Mask;
- ArrayRef<int> &CapturedMask;
-
- // capturing mask
- SDShuffle_match(const T0 &Op1, const T1 &Op2, ArrayRef<int> &MaskRef)
- : Op1(Op1), Op2(Op2), CapturedMask(MaskRef) {}
+ SDShuffle_match(const T0 &Op1, const T1 &Op2, const T2 &Mask)
+ : Op1(Op1), Op2(Op2), Mask(Mask) {}
template <typename MatchContext>
bool match(const MatchContext &Ctx, SDValue N) {
if (auto *I = dyn_cast<ShuffleVectorSDNode>(N)) {
- if (Op1.match(Ctx, I->getOperand(0)) &&
- Op2.match(Ctx, I->getOperand(1))) {
- CapturedMask = I->getMask();
- return true;
- }
+ return Op1.match(Ctx, I->getOperand(0)) &&
+ Op2.match(Ctx, I->getOperand(1)) && Mask.match(I->getMask());
}
return false;
}
};
-/// Matching against a specific match
-template <typename T0, typename T1> struct SDShuffle_maskMatch {
- T0 Op1;
- T1 Op2;
- ArrayRef<int> SpecificMask;
-
- SDShuffle_maskMatch(const T0 &Op1, const T1 &Op2, ArrayRef<int> Mask)
- : Op1(Op1), Op2(Op2), SpecificMask(Mask) {}
-
- template <typename MatchContext>
- bool match(const MatchContext &Ctx, SDValue N) {
- if (auto *I = dyn_cast<ShuffleVectorSDNode>(N)) {
- return Op1.match(Ctx, I->getOperand(0)) &&
- Op2.match(Ctx, I->getOperand(1)) &&
- std::equal(SpecificMask.begin(), SpecificMask.end(),
- I->getMask().begin(), I->getMask().end());
- }
- return false;
+struct m_Mask {
+ ArrayRef<int> &MaskRef;
+ m_Mask(ArrayRef<int> &MaskRef) : MaskRef(MaskRef) {}
+ bool match(ArrayRef<int> Mask) {
+ MaskRef = Mask;
+ return true;
}
};
+
+struct m_SpecificMask {
+ ArrayRef<int> &MaskRef;
----------------
mshockwave wrote:
I would recommend you to step back a little bit and think about the purpose of `m_Mask` and `m_SpecificMask`: in both IR PatternMatch and SD PatternMatch, the former one captures the mask, so you need to pass the ArrayRef that carries the _outgoing_ value by reference. Remember, reference in C++ is almost like a pointer, you write the captured mask to the underlying memory pointed by this reference in order to "export" the value.
The latter one, m_SpecificMask, is passing in a mask you want to _compare_ against with, so that's why it's pass by value.
Therefore, I think https://github.com/llvm/llvm-project/pull/119592/commits/80e64f6e30f1a5568bea6fbc62751f7c456d7e7c is wrong. And the CI passed because you also incorrectly changed the test from `ArrayRef<int> CaptureMask;` to `ArrayRef<int> CaptureMask(MaskData);` -- the `CaptureMask` here should be a variable storing the outgoing mask captured by m_Mask, so you really shouldn't initialize it.
https://github.com/llvm/llvm-project/pull/119592
More information about the llvm-commits
mailing list