[mlir][ArmSME] Remove func patterns from vector lowering

[matthias-springer]

Remove func.call and func.return patterns from populateArmSVELegalizeForLLVMExportPatterns. This function is called from ConvertVectorToLLVMPass::runOnOperation. That pass should lower only vector dialect ops, not func dialect ops. These patterns also seem to be unnecessary, as no test cases are failing without them. Also note that there is no func.func pattern, so any application of the above-mentioned patterns produces invalid IR.

[llvmbot]

@llvm/pr-subscribers-mlir-sve

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

Changes

Remove func.call and func.return patterns from populateArmSVELegalizeForLLVMExportPatterns. This function is called from ConvertVectorToLLVMPass::runOnOperation. That pass should lower vector dialect ops, not func dialect ops. These patterns also seem to be unnecessary, as no test cases are failing without them.

---

Full diff: https://github.com/llvm/llvm-project/pull/121640.diff

1 Files Affected:

• (modified) mlir/lib/Dialect/ArmSVE/Transforms/LegalizeForLLVMExport.cpp (-20)

diff --git a/mlir/lib/Dialect/ArmSVE/Transforms/LegalizeForLLVMExport.cpp b/mlir/lib/Dialect/ArmSVE/Transforms/LegalizeForLLVMExport.cpp
index 845a32c4d97b5e..2bdb640699d036 100644
--- a/mlir/lib/Dialect/ArmSVE/Transforms/LegalizeForLLVMExport.cpp
+++ b/mlir/lib/Dialect/ArmSVE/Transforms/LegalizeForLLVMExport.cpp
@@ -20,22 +20,6 @@
 using namespace mlir;
 using namespace mlir::arm_sve;
 
-template <typename OpTy>
-class ForwardOperands : public OpConversionPattern<OpTy> {
-  using OpConversionPattern<OpTy>::OpConversionPattern;
-
-  LogicalResult
-  matchAndRewrite(OpTy op, typename OpTy::Adaptor adaptor,
-                  ConversionPatternRewriter &rewriter) const final {
-    if (adaptor.getOperands().getTypes() == op->getOperands().getTypes())
-      return rewriter.notifyMatchFailure(op, "operand types already match");
-
-    rewriter.modifyOpInPlace(op,
-                             [&]() { op->setOperands(adaptor.getOperands()); });
-    return success();
-  }
-};
-
 using SdotOpLowering = OneToOneConvertToLLVMPattern<SdotOp, SdotIntrOp>;
 using SmmlaOpLowering = OneToOneConvertToLLVMPattern<SmmlaOp, SmmlaIntrOp>;
 using UdotOpLowering = OneToOneConvertToLLVMPattern<UdotOp, UdotIntrOp>;
@@ -204,10 +188,6 @@ void mlir::populateArmSVELegalizeForLLVMExportPatterns(
   // Populate conversion patterns
 
   // clang-format off
-  patterns.add<ForwardOperands<func::CallOp>,
-               ForwardOperands<func::CallIndirectOp>,
-               ForwardOperands<func::ReturnOp>>(converter,
-                                          &converter.getContext());
   patterns.add<SdotOpLowering,
                SmmlaOpLowering,
                UdotOpLowering,

[banach-space]

This pre-dates my involvement with SVE in MLIR, but makes total sense. LGTM and thanks for the clean-up 🙏🏻

These patterns also seem to be unnecessary, as no test cases are failing without them.

I suspect that you are skipping the SVE and SME integration tests? Those are DISABLED by default and also not included in pre-merge CI. Still, it should be safe to land this. I'd only recommend to track these bots that run the SVE+SME e2e tests:

• clang-aarch64-sve-vla
• clang-aarch64-sve-vls
• clang-aarch64-full-2stag