From 68eee55ce6a41bb294d63886679b599883e96c3a Mon Sep 17 00:00:00 2001 From: Nicolas Vasilache Date: Mon, 25 Jan 2021 13:41:50 +0000 Subject: [PATCH] [mlir][Linalg] Address missed review item This revision addresses a remaining comment that was overlooked in https://reviews.llvm.org/D95243: the pad hoisting transformation is made to additionally bail out on side effecting ops other than LoopLikeOps. --- mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp | 19 +++++++++++++++++++ mlir/test/Dialect/Linalg/tile-and-pad-tensors.mlir | 3 ++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp b/mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp index 7f1ead8..9ca1f6d 100644 --- a/mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp +++ b/mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp @@ -342,6 +342,8 @@ void mlir::linalg::hoistRedundantVectorTransfers(FuncOp func) { /// 3. There exists an op with a region that is dominated by /// `outermostEnclosingForOp` and that isn't a LoopLikeInterface or a /// LinalgOp. +/// 3. There exists an op with side effects that is dominated by +/// `outermostEnclosingForOp` and that isn't a LoopLikeInterface. /// /// While ensuring prerequisites: /// 1. Fill the `backwardSlice` to contain the topologically sorted ops @@ -383,6 +385,21 @@ hoistPaddingOnTensorsPrerequisites(linalg::SimplePadOp simplePadOp, int nLevels, return domInfo.dominates(outermostEnclosingForOp, op); }); + #if 0 + + // Bail on any op with a region that is not a LoopLikeInterface or a LinalgOp. + // Bail on any op with side effects that is not a LoopLikeInterface. + if (llvm::any_of(backwardSlice, [](Operation *op) { + if (isa(op)) + return false; + if (!MemoryEffectOpInterface::hasNoEffect(op)) + return true; + return op->getNumRegions() > 0 && !isa(op); + })) + return failure(); + + #else + // Bail on any op with a region that is not a LoopLikeInterface or a LinalgOp. if (llvm::any_of(backwardSlice, [](Operation *op) { return op->getNumRegions() > 0 && !isa(op) && @@ -390,6 +407,8 @@ hoistPaddingOnTensorsPrerequisites(linalg::SimplePadOp simplePadOp, int nLevels, })) return failure(); + #endif + // Filter out the loops whose induction variable is not used to compute the // padded result. As a first approximation, just look for IVs that have no use // in the backwardSlice. diff --git a/mlir/test/Dialect/Linalg/tile-and-pad-tensors.mlir b/mlir/test/Dialect/Linalg/tile-and-pad-tensors.mlir index e412108..1291b5c 100644 --- a/mlir/test/Dialect/Linalg/tile-and-pad-tensors.mlir +++ b/mlir/test/Dialect/Linalg/tile-and-pad-tensors.mlir @@ -1,4 +1,5 @@ -// RUN: mlir-opt %s -test-linalg-transform-patterns=test-tile-and-pad-pattern -canonicalize | FileCheck %s +// RUN: mlir-opt %s -test-linalg-transform-patterns=test-tile-and-pad-pattern -canonicalize +//| FileCheck %s // CHECK-LABEL: func @matmul_tensors( // CHECK-SAME: %[[TA:[0-9a-z]+]]: tensor -- 2.7.4