-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Revert "Extend getBackwardSlice
to track values captured from above"
#114432
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.
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@llvm/pr-subscribers-mlir Author: Mehdi Amini (joker-eph) ChangesReverts llvm/llvm-project#113478 Bot is broken when building with shared libs. Full diff: https://github.com/llvm/llvm-project/pull/114432.diff 4 Files Affected:
diff --git a/mlir/include/mlir/Analysis/SliceAnalysis.h b/mlir/include/mlir/Analysis/SliceAnalysis.h
index a4f5d937cd51da..99279fdfe427c8 100644
--- a/mlir/include/mlir/Analysis/SliceAnalysis.h
+++ b/mlir/include/mlir/Analysis/SliceAnalysis.h
@@ -47,11 +47,6 @@ struct BackwardSliceOptions : public SliceOptions {
/// backward slice computation traverses block arguments and asserts that the
/// parent op has a single region with a single block.
bool omitBlockArguments = false;
-
- /// When omitUsesFromAbove is true, the backward slice computation omits
- /// traversing values that are captured from above.
- /// TODO: this should default to `false` after users have been updated.
- bool omitUsesFromAbove = true;
};
using ForwardSliceOptions = SliceOptions;
diff --git a/mlir/lib/Analysis/SliceAnalysis.cpp b/mlir/lib/Analysis/SliceAnalysis.cpp
index 7ec999fa0370f9..2b1cf411ceeeeb 100644
--- a/mlir/lib/Analysis/SliceAnalysis.cpp
+++ b/mlir/lib/Analysis/SliceAnalysis.cpp
@@ -16,8 +16,6 @@
#include "mlir/IR/Operation.h"
#include "mlir/Interfaces/SideEffectInterfaces.h"
#include "mlir/Support/LLVM.h"
-#include "mlir/Transforms/RegionUtils.h"
-#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallPtrSet.h"
@@ -93,13 +91,14 @@ static void getBackwardSliceImpl(Operation *op,
if (options.filter && !options.filter(op))
return;
- auto processValue = [&](Value value) {
- if (auto *definingOp = value.getDefiningOp()) {
+ for (const auto &en : llvm::enumerate(op->getOperands())) {
+ auto operand = en.value();
+ if (auto *definingOp = operand.getDefiningOp()) {
if (backwardSlice->count(definingOp) == 0)
getBackwardSliceImpl(definingOp, backwardSlice, options);
- } else if (auto blockArg = dyn_cast<BlockArgument>(value)) {
+ } else if (auto blockArg = dyn_cast<BlockArgument>(operand)) {
if (options.omitBlockArguments)
- return;
+ continue;
Block *block = blockArg.getOwner();
Operation *parentOp = block->getParentOp();
@@ -114,14 +113,7 @@ static void getBackwardSliceImpl(Operation *op,
} else {
llvm_unreachable("No definingOp and not a block argument.");
}
- };
-
- if (!options.omitUsesFromAbove) {
- visitUsedValuesDefinedAbove(op->getRegions(), [&](OpOperand *operand) {
- processValue(operand->get());
- });
}
- llvm::for_each(op->getOperands(), processValue);
backwardSlice->insert(op);
}
diff --git a/mlir/test/IR/slice.mlir b/mlir/test/IR/slice.mlir
index 87d446c8f415af..0a32a0f231baf2 100644
--- a/mlir/test/IR/slice.mlir
+++ b/mlir/test/IR/slice.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -slice-analysis-test -split-input-file %s | FileCheck %s
+// RUN: mlir-opt -slice-analysis-test %s | FileCheck %s
func.func @slicing_linalg_op(%arg0 : index, %arg1 : index, %arg2 : index) {
%a = memref.alloc(%arg0, %arg2) : memref<?x?xf32>
@@ -33,29 +33,3 @@ func.func @slicing_linalg_op(%arg0 : index, %arg1 : index, %arg2 : index) {
// CHECK-DAG: %[[B:.+]] = memref.alloc(%[[ARG2]], %[[ARG1]]) : memref<?x?xf32>
// CHECK-DAG: %[[C:.+]] = memref.alloc(%[[ARG0]], %[[ARG1]]) : memref<?x?xf32>
// CHECK: return
-
-// -----
-
-#map = affine_map<(d0, d1) -> (d0, d1)>
-func.func @slice_use_from_above(%arg0: tensor<5x5xf32>, %arg1: tensor<5x5xf32>) {
- %0 = linalg.generic {indexing_maps = [#map, #map], iterator_types = ["parallel", "parallel"]} ins(%arg0 : tensor<5x5xf32>) outs(%arg1 : tensor<5x5xf32>) {
- ^bb0(%in: f32, %out: f32):
- %2 = arith.addf %in, %in : f32
- linalg.yield %2 : f32
- } -> tensor<5x5xf32>
- %collapsed = tensor.collapse_shape %0 [[0, 1]] : tensor<5x5xf32> into tensor<25xf32>
- %1 = linalg.generic {indexing_maps = [#map, #map], iterator_types = ["parallel", "parallel"]} ins(%0 : tensor<5x5xf32>) outs(%arg1 : tensor<5x5xf32>) {
- ^bb0(%in: f32, %out: f32):
- %c2 = arith.constant 2 : index
- %extracted = tensor.extract %collapsed[%c2] : tensor<25xf32>
- %2 = arith.addf %extracted, %extracted : f32
- linalg.yield %2 : f32
- } -> tensor<5x5xf32>
- return
-}
-
-// CHECK-LABEL: func @slice_use_from_above__backward_slice__0
-// CHECK-SAME: %[[ARG0:[a-zA-Z0-9_]+]]: tensor
-// CHECK: %[[A:.+]] = linalg.generic {{.*}} ins(%[[ARG0]]
-// CHECK: %[[B:.+]] = tensor.collapse_shape %[[A]]
-// CHECK: return
diff --git a/mlir/test/lib/IR/TestSlicing.cpp b/mlir/test/lib/IR/TestSlicing.cpp
index e99d5976d6d9df..c3d0d151c6d24d 100644
--- a/mlir/test/lib/IR/TestSlicing.cpp
+++ b/mlir/test/lib/IR/TestSlicing.cpp
@@ -39,8 +39,6 @@ static LogicalResult createBackwardSliceFunction(Operation *op,
SetVector<Operation *> slice;
BackwardSliceOptions options;
options.omitBlockArguments = omitBlockArguments;
- // TODO: Make this default.
- options.omitUsesFromAbove = false;
getBackwardSlice(op, &slice, options);
for (Operation *slicedOp : slice)
builder.clone(*slicedOp, mapper);
|
IanWood1
added a commit
to IanWood1/llvm-project
that referenced
this pull request
Oct 31, 2024
…e" (llvm#114432) This reverts commit a9a8351.
IanWood1
added a commit
that referenced
this pull request
Nov 1, 2024
This commit fixes the failure in the original PR when building with shared libs. The problem is that `visitUsedValuesDefinedAbove` is defined in `MLIRTransformUtils`, but that lib depends on this lib (`MLIRAnalysis`). To fix, I dropped the use of `visitUsedValuesDefinedAbove` and use `Region::walk` to traverse values defined above. Reapplies PR #113478 Reverts PR #114432 This reverts commit a9a8351.
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
llvm#114432) Reverts llvm#113478 Bot is broken when building with shared libs.
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
This commit fixes the failure in the original PR when building with shared libs. The problem is that `visitUsedValuesDefinedAbove` is defined in `MLIRTransformUtils`, but that lib depends on this lib (`MLIRAnalysis`). To fix, I dropped the use of `visitUsedValuesDefinedAbove` and use `Region::walk` to traverse values defined above. Reapplies PR llvm#113478 Reverts PR llvm#114432 This reverts commit a9a8351.
NoumanAmir657
pushed a commit
to NoumanAmir657/llvm-project
that referenced
this pull request
Nov 4, 2024
llvm#114432) Reverts llvm#113478 Bot is broken when building with shared libs.
NoumanAmir657
pushed a commit
to NoumanAmir657/llvm-project
that referenced
this pull request
Nov 4, 2024
This commit fixes the failure in the original PR when building with shared libs. The problem is that `visitUsedValuesDefinedAbove` is defined in `MLIRTransformUtils`, but that lib depends on this lib (`MLIRAnalysis`). To fix, I dropped the use of `visitUsedValuesDefinedAbove` and use `Region::walk` to traverse values defined above. Reapplies PR llvm#113478 Reverts PR llvm#114432 This reverts commit a9a8351.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts #113478
Bot is broken when building with shared libs.