[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
Mon Dec 30 10:33:40 PST 2024


================
@@ -547,6 +547,39 @@ struct BinaryOpc_match {
   }
 };
 
+/// Matching while capturing mask
+template <typename T0, typename T1, typename T2> struct SDShuffle_match {
+  T0 Op1;
+  T1 Op2;
+  T2 Mask;
+
+  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)) {
+      return Op1.match(Ctx, I->getOperand(0)) &&
+             Op2.match(Ctx, I->getOperand(1)) && Mask.match(I->getMask());
+    }
+    return false;
+  }
+};
+struct m_Mask {
+  ArrayRef<int> MaskRef;
----------------
mshockwave wrote:

> In the IR, patternmatch.h, both m_Mask and m_SpecificMask are implement as pass-by-reference (as is m_SplatOrPoisonMask, m_ZeroMask is by value)

IR PatternMatch's m_SpecificMask is wrong (though it doesn't affect the correctness) because if you look info it, [it never assigns](https://github.com/llvm/llvm-project/blob/af83093933ca73bc82c33130f8bda9f1ae54aae2/llvm/include/llvm/IR/PatternMatch.h#L1849) value into MaskRef.

>  passing an ArrayRef by reference or by value seems almost the same,

It's functionally the same for both SDPatternMatch's and IR PatternMatch's m_SpecificMatch (though from a software engineering point of view, you should pass by value for m_SpecificMatch), but it's not the same for m_Mask: for m_Mask, you want to capture the mask value so you _have_ to use a reference (or pointer) to carry the result.

> In both cases you have access to the underlying array, so modifcations can be made

No, ArrayRef is [designed](https://github.com/llvm/llvm-project/blob/65a2eb0b1589590ae78cc1e5f05cd004b3b3bec5/llvm/include/llvm/ADT/ArrayRef.h#L29) to be a reference to an _immutable_ chunk of memory -- otherwise `ArrayRef::operator[]` should have a return type of `T&` instead of `const T&` --  so you cannot modify the underlying array. If you want to do that, there is `llvm::MutableArrayRef`.

https://github.com/llvm/llvm-project/pull/119592


More information about the llvm-commits mailing list