[llvm-branch-commits] [mlir] [MLIR][LLVM]: Add an IR utility to perform slice walking (PR #103053)
Christian Ulmann via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Aug 14 07:45:30 PDT 2024
https://github.com/Dinistro updated https://github.com/llvm/llvm-project/pull/103053
>From a9fd34956d7a9ddfc06bcfaf011cec7e3c5ddcd2 Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Tue, 13 Aug 2024 12:46:30 +0000
Subject: [PATCH 1/6] MLIR, LLVM: Add an IR utility to perform proper slicing
This commit introduces a backwards slicing utility that is both
`RegionBranchOpInterface` and `BranchOpInterface` aware. This is a first
step in replacing the currently broken slicing utilities that cannot be
used for structured control flow.
Note that the utility is currently placed in the IR library, because
LLVM's inlining uses it and is part of the LLVMIR library. There was
originally a substantial push back on moving the inliner interface out
of said library.
---
mlir/include/mlir/IR/SliceSupport.h | 123 +++++++++++++++++
.../Transforms/InlinerInterfaceImpl.cpp | 85 ++----------
mlir/lib/IR/CMakeLists.txt | 1 +
mlir/lib/IR/SliceSupport.cpp | 130 ++++++++++++++++++
4 files changed, 268 insertions(+), 71 deletions(-)
create mode 100644 mlir/include/mlir/IR/SliceSupport.h
create mode 100644 mlir/lib/IR/SliceSupport.cpp
diff --git a/mlir/include/mlir/IR/SliceSupport.h b/mlir/include/mlir/IR/SliceSupport.h
new file mode 100644
index 00000000000000..b840ca72d9d027
--- /dev/null
+++ b/mlir/include/mlir/IR/SliceSupport.h
@@ -0,0 +1,123 @@
+//===- SliceSupport.h - Helpers for performing IR slicing -------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_IR_SLICESUPPORT_H
+#define MLIR_IR_SLICESUPPORT_H
+
+#include "mlir/IR/ValueRange.h"
+
+namespace mlir {
+
+/// A class to signal how to proceed with the walk of the backward slice:
+/// - Interrupt: Stops the walk.
+/// - Advance: Continues the walk to control flow predecessors values.
+/// - AdvanceTo: Continues the walk to user-specified values.
+/// - Skip: Continues the walk, but skips the predecessors of the current value.
+class WalkContinuation {
+public:
+ enum class WalkAction {
+ /// Stops the walk.
+ Interrupt,
+ /// Continues the walk to control flow predecessors values.
+ Advance,
+ /// Continues the walk to user-specified values.
+ AdvanceTo,
+ /// Continues the walk, but skips the predecessors of the current value.
+ Skip
+ };
+
+ WalkContinuation(WalkAction action, mlir::ValueRange nextValues)
+ : action(action), nextValues(nextValues) {}
+
+ /// Allows LogicalResult to interrupt the walk on failure.
+ explicit WalkContinuation(llvm::LogicalResult action)
+ : action(failed(action) ? WalkAction::Interrupt : WalkAction::Advance) {}
+
+ /// Allows diagnostics to interrupt the walk.
+ explicit WalkContinuation(mlir::Diagnostic &&)
+ : action(WalkAction::Interrupt) {}
+
+ /// Allows diagnostics to interrupt the walk.
+ explicit WalkContinuation(mlir::InFlightDiagnostic &&)
+ : action(WalkAction::Interrupt) {}
+
+ /// Creates a continuation that interrupts the walk.
+ static WalkContinuation interrupt() {
+ return WalkContinuation(WalkAction::Interrupt, {});
+ }
+
+ /// Creates a continuation that adds the user-specified `nextValues` to the
+ /// work list and advances the walk. Unlike advance, this function does not
+ /// add the control flow predecessor values to the work list.
+ static WalkContinuation advanceTo(mlir::ValueRange nextValues) {
+ return WalkContinuation(WalkAction::AdvanceTo, nextValues);
+ }
+
+ /// Creates a continuation that adds the control flow predecessor values to
+ /// the work list and advances the walk.
+ static WalkContinuation advance() {
+ return WalkContinuation(WalkAction::Advance, {});
+ }
+
+ /// Creates a continuation that advances the walk without adding any
+ /// predecessor values to the work list.
+ static WalkContinuation skip() {
+ return WalkContinuation(WalkAction::Skip, {});
+ }
+
+ /// Returns true if the walk was interrupted.
+ bool wasInterrupted() const { return action == WalkAction::Interrupt; }
+
+ /// Returns true if the walk was skipped.
+ bool wasSkipped() const { return action == WalkAction::Skip; }
+
+ /// Returns true if the walk was advanced to user-specified values.
+ bool wasAdvancedTo() const { return action == WalkAction::AdvanceTo; }
+
+ /// Returns the next values to continue the walk with.
+ mlir::ArrayRef<mlir::Value> getNextValues() const { return nextValues; }
+
+private:
+ WalkAction action;
+ /// The next values to continue the walk with.
+ mlir::SmallVector<mlir::Value> nextValues;
+};
+
+/// A callback that is invoked for each value encountered during the walk of the
+/// backward slice. The callback takes the current value, and returns the walk
+/// continuation, which determines if the walk should proceed and if yes, with
+/// which values.
+using WalkCallback = mlir::function_ref<WalkContinuation(mlir::Value)>;
+
+/// Walks the backward slice starting from the `rootValues` using a depth-first
+/// traversal following the use-def chains. The walk calls the provided
+/// `walkCallback` for each value encountered in the backward slice and uses the
+/// returned walk continuation to determine how to proceed. Additionally, the
+/// walk also transparently traverses through select operations and control flow
+/// operations that implement RegionBranchOpInterface or BranchOpInterface.
+WalkContinuation walkBackwardSlice(mlir::ValueRange rootValues,
+ WalkCallback walkCallback);
+
+/// A callback that is invoked for each value encountered during the walk of the
+/// backward slice. The callback takes the current value, and returns the walk
+/// continuation, which determines if the walk should proceed and if yes, with
+/// which values.
+using WalkCallback = mlir::function_ref<WalkContinuation(mlir::Value)>;
+
+/// Walks the backward slice starting from the `rootValues` using a depth-first
+/// traversal following the use-def chains. The walk calls the provided
+/// `walkCallback` for each value encountered in the backward slice and uses the
+/// returned walk continuation to determine how to proceed. Additionally, the
+/// walk also transparently traverses through select operations and control flow
+/// operations that implement RegionBranchOpInterface or BranchOpInterface.
+WalkContinuation walkBackwardSlice(mlir::ValueRange rootValues,
+ WalkCallback walkCallback);
+
+} // namespace mlir
+
+#endif // MLIR_IR_SLICESUPPORT_H
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
index 8eba76a9abee8d..3a3aa7b7470e83 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
@@ -14,6 +14,7 @@
#include "mlir/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.h"
#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
#include "mlir/IR/Matchers.h"
+#include "mlir/IR/SliceSupport.h"
#include "mlir/Interfaces/DataLayoutInterfaces.h"
#include "mlir/Transforms/InliningUtils.h"
#include "llvm/ADT/ScopeExit.h"
@@ -221,86 +222,28 @@ static ArrayAttr concatArrayAttr(ArrayAttr lhs, ArrayAttr rhs) {
return ArrayAttr::get(lhs.getContext(), result);
}
-/// Attempts to return the underlying pointer value that `pointerValue` is based
-/// on. This traverses down the chain of operations to the last operation
-/// producing the base pointer and returns it. If it encounters an operation it
-/// cannot further traverse through, returns the operation's result.
-static Value getUnderlyingObject(Value pointerValue) {
- while (true) {
- if (auto gepOp = pointerValue.getDefiningOp<LLVM::GEPOp>()) {
- pointerValue = gepOp.getBase();
- continue;
- }
-
- if (auto addrCast = pointerValue.getDefiningOp<LLVM::AddrSpaceCastOp>()) {
- pointerValue = addrCast.getOperand();
- continue;
- }
-
- break;
- }
-
- return pointerValue;
-}
-
/// Attempts to return the set of all underlying pointer values that
/// `pointerValue` is based on. This function traverses through select
/// operations and block arguments unlike getUnderlyingObject.
static SmallVector<Value> getUnderlyingObjectSet(Value pointerValue) {
SmallVector<Value> result;
+ walkBackwardSlice(pointerValue, [&](Value val) {
+ if (auto gepOp = val.getDefiningOp<LLVM::GEPOp>())
+ return WalkContinuation::advanceTo(gepOp.getBase());
- SmallVector<Value> workList{pointerValue};
- // Avoid dataflow loops.
- SmallPtrSet<Value, 4> seen;
- do {
- Value current = workList.pop_back_val();
- current = getUnderlyingObject(current);
-
- if (!seen.insert(current).second)
- continue;
-
- if (auto selectOp = current.getDefiningOp<LLVM::SelectOp>()) {
- workList.push_back(selectOp.getTrueValue());
- workList.push_back(selectOp.getFalseValue());
- continue;
- }
-
- if (auto blockArg = dyn_cast<BlockArgument>(current)) {
- Block *parentBlock = blockArg.getParentBlock();
-
- // Attempt to find all block argument operands for every predecessor.
- // If any operand to the block argument wasn't found in a predecessor,
- // conservatively add the block argument to the result set.
- SmallVector<Value> operands;
- bool anyUnknown = false;
- for (auto iter = parentBlock->pred_begin();
- iter != parentBlock->pred_end(); iter++) {
- auto branch = dyn_cast<BranchOpInterface>((*iter)->getTerminator());
- if (!branch) {
- result.push_back(blockArg);
- anyUnknown = true;
- break;
- }
-
- Value operand = branch.getSuccessorOperands(
- iter.getSuccessorIndex())[blockArg.getArgNumber()];
- if (!operand) {
- result.push_back(blockArg);
- anyUnknown = true;
- break;
- }
-
- operands.push_back(operand);
- }
+ if (auto addrCast = val.getDefiningOp<LLVM::AddrSpaceCastOp>())
+ return WalkContinuation::advanceTo(addrCast.getOperand());
- if (!anyUnknown)
- llvm::append_range(workList, operands);
+ // TODO: Add a SelectLikeOpInterface and use it in the slicing utility.
+ if (auto selectOp = val.getDefiningOp<LLVM::SelectOp>())
+ return WalkContinuation::advanceTo(
+ {selectOp.getTrueValue(), selectOp.getFalseValue()});
- continue;
- }
+ if (isa<OpResult>(val))
+ result.push_back(val);
- result.push_back(current);
- } while (!workList.empty());
+ return WalkContinuation::advance();
+ });
return result;
}
diff --git a/mlir/lib/IR/CMakeLists.txt b/mlir/lib/IR/CMakeLists.txt
index c38ce6c058a006..bfff29abc824a0 100644
--- a/mlir/lib/IR/CMakeLists.txt
+++ b/mlir/lib/IR/CMakeLists.txt
@@ -32,6 +32,7 @@ add_mlir_library(MLIRIR
PatternMatch.cpp
Region.cpp
RegionKindInterface.cpp
+ SliceSupport.cpp
SymbolTable.cpp
TensorEncoding.cpp
Types.cpp
diff --git a/mlir/lib/IR/SliceSupport.cpp b/mlir/lib/IR/SliceSupport.cpp
new file mode 100644
index 00000000000000..b3bb6a39ddd981
--- /dev/null
+++ b/mlir/lib/IR/SliceSupport.cpp
@@ -0,0 +1,130 @@
+#include "mlir/IR/SliceSupport.h"
+#include "mlir/Interfaces/ControlFlowInterfaces.h"
+
+using namespace mlir;
+
+/// Returns the operands from all predecessor regions that match `operandNumber`
+/// for the `successor` region within `regionOp`.
+static SmallVector<Value>
+getRegionPredecessorOperands(RegionBranchOpInterface regionOp,
+ RegionSuccessor successor,
+ unsigned operandNumber) {
+ SmallVector<Value> predecessorOperands;
+
+ // Returns true if `successors` contains `successor`.
+ auto isContained = [](ArrayRef<RegionSuccessor> successors,
+ RegionSuccessor successor) {
+ auto *it = llvm::find_if(successors, [&successor](RegionSuccessor curr) {
+ return curr.getSuccessor() == successor.getSuccessor();
+ });
+ return it != successors.end();
+ };
+
+ // Search the operand ranges on the region operation itself.
+ SmallVector<Attribute> operandAttributes(regionOp->getNumOperands());
+ SmallVector<RegionSuccessor> successors;
+ regionOp.getEntrySuccessorRegions(operandAttributes, successors);
+ if (isContained(successors, successor)) {
+ OperandRange operands = regionOp.getEntrySuccessorOperands(successor);
+ predecessorOperands.push_back(operands[operandNumber]);
+ }
+
+ // Search the operand ranges on region terminators.
+ for (Region ®ion : regionOp->getRegions()) {
+ for (Block &block : region) {
+ auto terminatorOp =
+ dyn_cast<RegionBranchTerminatorOpInterface>(block.getTerminator());
+ if (!terminatorOp)
+ continue;
+ SmallVector<Attribute> operandAttributes(terminatorOp->getNumOperands());
+ SmallVector<RegionSuccessor> successors;
+ terminatorOp.getSuccessorRegions(operandAttributes, successors);
+ if (isContained(successors, successor)) {
+ OperandRange operands = terminatorOp.getSuccessorOperands(successor);
+ predecessorOperands.push_back(operands[operandNumber]);
+ }
+ }
+ }
+
+ return predecessorOperands;
+}
+
+/// Returns the predecessor branch operands that match `blockArg`.
+static SmallVector<Value> getBlockPredecessorOperands(BlockArgument blockArg) {
+ Block *block = blockArg.getOwner();
+
+ // Search the predecessor operands for all predecessor terminators.
+ SmallVector<Value> predecessorOperands;
+ for (auto it = block->pred_begin(); it != block->pred_end(); ++it) {
+ Block *predecessor = *it;
+ auto branchOp = cast<BranchOpInterface>(predecessor->getTerminator());
+ SuccessorOperands successorOperands =
+ branchOp.getSuccessorOperands(it.getSuccessorIndex());
+ // Store the predecessor operand if the block argument matches an operand
+ // and is not produced by the terminator.
+ if (Value operand = successorOperands[blockArg.getArgNumber()])
+ predecessorOperands.push_back(operand);
+ }
+
+ return predecessorOperands;
+}
+
+mlir::WalkContinuation mlir::walkBackwardSlice(ValueRange rootValues,
+ WalkCallback walkCallback) {
+ // Search the backward slice starting from the root values.
+ SmallVector<Value> workList = rootValues;
+ llvm::SmallDenseSet<Value, 16> seenValues;
+ while (!workList.empty()) {
+ // Search the backward slice of the current value.
+ Value current = workList.pop_back_val();
+
+ // Skip the current value if it has already been seen.
+ if (!seenValues.insert(current).second)
+ continue;
+
+ // Call the walk callback with the current value.
+ WalkContinuation continuation = walkCallback(current);
+ if (continuation.wasInterrupted())
+ return continuation;
+ if (continuation.wasSkipped())
+ continue;
+
+ // Add the next values to the work list if the walk should continue.
+ if (continuation.wasAdvancedTo()) {
+ workList.append(continuation.getNextValues().begin(),
+ continuation.getNextValues().end());
+ continue;
+ }
+
+ // Add the control flow predecessor operands to the work list.
+ if (OpResult opResult = dyn_cast<OpResult>(current)) {
+ auto regionOp = dyn_cast<RegionBranchOpInterface>(opResult.getOwner());
+ if (!regionOp)
+ continue;
+ RegionSuccessor region(regionOp->getResults());
+ SmallVector<Value> predecessorOperands = getRegionPredecessorOperands(
+ regionOp, region, opResult.getResultNumber());
+ workList.append(predecessorOperands.begin(), predecessorOperands.end());
+ continue;
+ }
+
+ auto blockArg = cast<BlockArgument>(current);
+ Block *block = blockArg.getOwner();
+ // Search the region predecessor operands for structured control flow.
+ auto regionBranchOp =
+ dyn_cast<RegionBranchOpInterface>(block->getParentOp());
+ if (block->isEntryBlock() && regionBranchOp) {
+ RegionSuccessor region(blockArg.getParentRegion());
+ SmallVector<Value> predecessorOperands = getRegionPredecessorOperands(
+ regionBranchOp, region, blockArg.getArgNumber());
+ workList.append(predecessorOperands.begin(), predecessorOperands.end());
+ continue;
+ }
+ // Search the block predecessor operands for unstructured control flow.
+ SmallVector<Value> predecessorOperands =
+ getBlockPredecessorOperands(blockArg);
+ workList.append(predecessorOperands.begin(), predecessorOperands.end());
+ }
+
+ return WalkContinuation::advance();
+}
>From a5c512d3d81621efb2db7c1d8884df8188eb79b2 Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Wed, 14 Aug 2024 11:14:22 +0000
Subject: [PATCH 2/6] turn into a general slice walk
---
mlir/include/mlir/IR/SliceSupport.h | 50 ++------
.../Transforms/InlinerInterfaceImpl.cpp | 36 ++++--
mlir/lib/IR/SliceSupport.cpp | 116 ++++++++++--------
3 files changed, 105 insertions(+), 97 deletions(-)
diff --git a/mlir/include/mlir/IR/SliceSupport.h b/mlir/include/mlir/IR/SliceSupport.h
index b840ca72d9d027..a28b24ffcb3911 100644
--- a/mlir/include/mlir/IR/SliceSupport.h
+++ b/mlir/include/mlir/IR/SliceSupport.h
@@ -15,7 +15,6 @@ namespace mlir {
/// A class to signal how to proceed with the walk of the backward slice:
/// - Interrupt: Stops the walk.
-/// - Advance: Continues the walk to control flow predecessors values.
/// - AdvanceTo: Continues the walk to user-specified values.
/// - Skip: Continues the walk, but skips the predecessors of the current value.
class WalkContinuation {
@@ -23,8 +22,6 @@ class WalkContinuation {
enum class WalkAction {
/// Stops the walk.
Interrupt,
- /// Continues the walk to control flow predecessors values.
- Advance,
/// Continues the walk to user-specified values.
AdvanceTo,
/// Continues the walk, but skips the predecessors of the current value.
@@ -34,10 +31,6 @@ class WalkContinuation {
WalkContinuation(WalkAction action, mlir::ValueRange nextValues)
: action(action), nextValues(nextValues) {}
- /// Allows LogicalResult to interrupt the walk on failure.
- explicit WalkContinuation(llvm::LogicalResult action)
- : action(failed(action) ? WalkAction::Interrupt : WalkAction::Advance) {}
-
/// Allows diagnostics to interrupt the walk.
explicit WalkContinuation(mlir::Diagnostic &&)
: action(WalkAction::Interrupt) {}
@@ -58,12 +51,6 @@ class WalkContinuation {
return WalkContinuation(WalkAction::AdvanceTo, nextValues);
}
- /// Creates a continuation that adds the control flow predecessor values to
- /// the work list and advances the walk.
- static WalkContinuation advance() {
- return WalkContinuation(WalkAction::Advance, {});
- }
-
/// Creates a continuation that advances the walk without adding any
/// predecessor values to the work list.
static WalkContinuation skip() {
@@ -89,34 +76,23 @@ class WalkContinuation {
};
/// A callback that is invoked for each value encountered during the walk of the
-/// backward slice. The callback takes the current value, and returns the walk
-/// continuation, which determines if the walk should proceed and if yes, with
-/// which values.
-using WalkCallback = mlir::function_ref<WalkContinuation(mlir::Value)>;
-
-/// Walks the backward slice starting from the `rootValues` using a depth-first
-/// traversal following the use-def chains. The walk calls the provided
-/// `walkCallback` for each value encountered in the backward slice and uses the
-/// returned walk continuation to determine how to proceed. Additionally, the
-/// walk also transparently traverses through select operations and control flow
-/// operations that implement RegionBranchOpInterface or BranchOpInterface.
-WalkContinuation walkBackwardSlice(mlir::ValueRange rootValues,
- WalkCallback walkCallback);
-
-/// A callback that is invoked for each value encountered during the walk of the
-/// backward slice. The callback takes the current value, and returns the walk
+/// slice. The callback takes the current value, and returns the walk
/// continuation, which determines if the walk should proceed and if yes, with
/// which values.
using WalkCallback = mlir::function_ref<WalkContinuation(mlir::Value)>;
-/// Walks the backward slice starting from the `rootValues` using a depth-first
-/// traversal following the use-def chains. The walk calls the provided
-/// `walkCallback` for each value encountered in the backward slice and uses the
-/// returned walk continuation to determine how to proceed. Additionally, the
-/// walk also transparently traverses through select operations and control flow
-/// operations that implement RegionBranchOpInterface or BranchOpInterface.
-WalkContinuation walkBackwardSlice(mlir::ValueRange rootValues,
- WalkCallback walkCallback);
+/// Walks the slice starting from the `rootValues` using a depth-first
+/// traversal. The walk calls the provided `walkCallback` for each value
+/// encountered in the slice and uses the returned walk continuation to
+/// determine how to proceed.
+WalkContinuation walkSlice(mlir::ValueRange rootValues,
+ WalkCallback walkCallback);
+
+/// Computes a vector of all control predecessors of `value`. Relies on
+/// RegionBranchOpInterface and BranchOpInterface to determine predecessors.
+/// Returns nullopt if value has no predecessors or when the relevant operations
+/// are missing the interface implementations.
+std::optional<SmallVector<Value>> getControlFlowPredecessors(Value value);
} // namespace mlir
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
index 3a3aa7b7470e83..cba9460bf92569 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
@@ -224,10 +224,11 @@ static ArrayAttr concatArrayAttr(ArrayAttr lhs, ArrayAttr rhs) {
/// Attempts to return the set of all underlying pointer values that
/// `pointerValue` is based on. This function traverses through select
-/// operations and block arguments unlike getUnderlyingObject.
-static SmallVector<Value> getUnderlyingObjectSet(Value pointerValue) {
+/// operations and block arguments.
+static FailureOr<SmallVector<Value>>
+getUnderlyingObjectSet(Value pointerValue) {
SmallVector<Value> result;
- walkBackwardSlice(pointerValue, [&](Value val) {
+ WalkContinuation walkResult = walkSlice(pointerValue, [&](Value val) {
if (auto gepOp = val.getDefiningOp<LLVM::GEPOp>())
return WalkContinuation::advanceTo(gepOp.getBase());
@@ -239,12 +240,28 @@ static SmallVector<Value> getUnderlyingObjectSet(Value pointerValue) {
return WalkContinuation::advanceTo(
{selectOp.getTrueValue(), selectOp.getFalseValue()});
- if (isa<OpResult>(val))
+ // Attempt to advance to control flow predecessors.
+ std::optional<SmallVector<Value>> controlFlowPredecessors =
+ getControlFlowPredecessors(val);
+ if (controlFlowPredecessors)
+ return WalkContinuation::advanceTo(*controlFlowPredecessors);
+
+ // For all non-control flow results, consider `val` an underlying object.
+ if (isa<OpResult>(val)) {
result.push_back(val);
+ return WalkContinuation::skip();
+ }
- return WalkContinuation::advance();
+ // If this place is reached, `val` is a block argument that is not
+ // understood. Therfore, we conservatively interrupt.
+ // Note: Dealing with the function arguments is not necessary, as the slice
+ // would have to go through an SSACopyOp first.
+ return WalkContinuation::interrupt();
});
+ if (walkResult.wasInterrupted())
+ return failure();
+
return result;
}
@@ -306,9 +323,14 @@ static void createNewAliasScopesFromNoAliasParameter(
// Find the set of underlying pointers that this pointer is based on.
SmallPtrSet<Value, 4> basedOnPointers;
- for (Value pointer : pointerArgs)
- llvm::copy(getUnderlyingObjectSet(pointer),
+ for (Value pointer : pointerArgs) {
+ FailureOr<SmallVector<Value>> underlyingObjectSet =
+ getUnderlyingObjectSet(pointer);
+ if (failed(underlyingObjectSet))
+ return;
+ llvm::copy(*underlyingObjectSet,
std::inserter(basedOnPointers, basedOnPointers.begin()));
+ }
bool aliasesOtherKnownObject = false;
// Go through the based on pointers and check that they are either:
diff --git a/mlir/lib/IR/SliceSupport.cpp b/mlir/lib/IR/SliceSupport.cpp
index b3bb6a39ddd981..0c4d4e15fe83cf 100644
--- a/mlir/lib/IR/SliceSupport.cpp
+++ b/mlir/lib/IR/SliceSupport.cpp
@@ -3,6 +3,35 @@
using namespace mlir;
+WalkContinuation mlir::walkSlice(ValueRange rootValues,
+ WalkCallback walkCallback) {
+ // Search the backward slice starting from the root values.
+ SmallVector<Value> workList = rootValues;
+ llvm::SmallDenseSet<Value, 16> seenValues;
+ while (!workList.empty()) {
+ // Search the backward slice of the current value.
+ Value current = workList.pop_back_val();
+
+ // Skip the current value if it has already been seen.
+ if (!seenValues.insert(current).second)
+ continue;
+
+ // Call the walk callback with the current value.
+ WalkContinuation continuation = walkCallback(current);
+ if (continuation.wasInterrupted())
+ return continuation;
+ if (continuation.wasSkipped())
+ continue;
+
+ assert(continuation.wasAdvancedTo());
+ // Add the next values to the work list if the walk should continue.
+ workList.append(continuation.getNextValues().begin(),
+ continuation.getNextValues().end());
+ }
+
+ return WalkContinuation::skip();
+}
+
/// Returns the operands from all predecessor regions that match `operandNumber`
/// for the `successor` region within `regionOp`.
static SmallVector<Value>
@@ -49,15 +78,20 @@ getRegionPredecessorOperands(RegionBranchOpInterface regionOp,
return predecessorOperands;
}
-/// Returns the predecessor branch operands that match `blockArg`.
-static SmallVector<Value> getBlockPredecessorOperands(BlockArgument blockArg) {
+/// Returns the predecessor branch operands that match `blockArg`. Returns a
+/// nullopt when some of the predecessor terminators do not implement the
+/// BranchOpInterface.
+static std::optional<SmallVector<Value>>
+getBlockPredecessorOperands(BlockArgument blockArg) {
Block *block = blockArg.getOwner();
// Search the predecessor operands for all predecessor terminators.
SmallVector<Value> predecessorOperands;
for (auto it = block->pred_begin(); it != block->pred_end(); ++it) {
Block *predecessor = *it;
- auto branchOp = cast<BranchOpInterface>(predecessor->getTerminator());
+ auto branchOp = dyn_cast<BranchOpInterface>(predecessor->getTerminator());
+ if (!branchOp)
+ return std::nullopt;
SuccessorOperands successorOperands =
branchOp.getSuccessorOperands(it.getSuccessorIndex());
// Store the predecessor operand if the block argument matches an operand
@@ -69,62 +103,38 @@ static SmallVector<Value> getBlockPredecessorOperands(BlockArgument blockArg) {
return predecessorOperands;
}
-mlir::WalkContinuation mlir::walkBackwardSlice(ValueRange rootValues,
- WalkCallback walkCallback) {
- // Search the backward slice starting from the root values.
- SmallVector<Value> workList = rootValues;
- llvm::SmallDenseSet<Value, 16> seenValues;
- while (!workList.empty()) {
- // Search the backward slice of the current value.
- Value current = workList.pop_back_val();
-
- // Skip the current value if it has already been seen.
- if (!seenValues.insert(current).second)
- continue;
-
- // Call the walk callback with the current value.
- WalkContinuation continuation = walkCallback(current);
- if (continuation.wasInterrupted())
- return continuation;
- if (continuation.wasSkipped())
- continue;
-
- // Add the next values to the work list if the walk should continue.
- if (continuation.wasAdvancedTo()) {
- workList.append(continuation.getNextValues().begin(),
- continuation.getNextValues().end());
- continue;
- }
-
+std::optional<SmallVector<Value>>
+mlir::getControlFlowPredecessors(Value value) {
+ SmallVector<Value> result;
+ if (OpResult opResult = dyn_cast<OpResult>(value)) {
+ auto regionOp = dyn_cast<RegionBranchOpInterface>(opResult.getOwner());
+ // If the interface is not implemented, there are no control flow
+ // predecessors to work with.
+ if (!regionOp)
+ return std::nullopt;
// Add the control flow predecessor operands to the work list.
- if (OpResult opResult = dyn_cast<OpResult>(current)) {
- auto regionOp = dyn_cast<RegionBranchOpInterface>(opResult.getOwner());
- if (!regionOp)
- continue;
- RegionSuccessor region(regionOp->getResults());
- SmallVector<Value> predecessorOperands = getRegionPredecessorOperands(
- regionOp, region, opResult.getResultNumber());
- workList.append(predecessorOperands.begin(), predecessorOperands.end());
- continue;
- }
+ RegionSuccessor region(regionOp->getResults());
+ SmallVector<Value> predecessorOperands = getRegionPredecessorOperands(
+ regionOp, region, opResult.getResultNumber());
+ return predecessorOperands;
+ }
- auto blockArg = cast<BlockArgument>(current);
- Block *block = blockArg.getOwner();
- // Search the region predecessor operands for structured control flow.
- auto regionBranchOp =
- dyn_cast<RegionBranchOpInterface>(block->getParentOp());
- if (block->isEntryBlock() && regionBranchOp) {
+ auto blockArg = cast<BlockArgument>(value);
+ Block *block = blockArg.getOwner();
+ // Search the region predecessor operands for structured control flow.
+ if (block->isEntryBlock()) {
+ if (auto regionBranchOp =
+ dyn_cast<RegionBranchOpInterface>(block->getParentOp())) {
RegionSuccessor region(blockArg.getParentRegion());
SmallVector<Value> predecessorOperands = getRegionPredecessorOperands(
regionBranchOp, region, blockArg.getArgNumber());
- workList.append(predecessorOperands.begin(), predecessorOperands.end());
- continue;
+ return predecessorOperands;
}
- // Search the block predecessor operands for unstructured control flow.
- SmallVector<Value> predecessorOperands =
- getBlockPredecessorOperands(blockArg);
- workList.append(predecessorOperands.begin(), predecessorOperands.end());
+ // Unclear how to deal with this operation, conservatively return a
+ // failure.
+ return std::nullopt;
}
- return WalkContinuation::advance();
+ // Search the block predecessor operands for unstructured control flow.
+ return getBlockPredecessorOperands(blockArg);
}
>From 10ee77feb18c3a88dc5363aa7e92df6dd5c2f492 Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Wed, 14 Aug 2024 11:44:08 +0000
Subject: [PATCH 3/6] add llvm region inlining test
---
.../Dialect/LLVMIR/inlining-alias-scopes.mlir | 33 +++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/mlir/test/Dialect/LLVMIR/inlining-alias-scopes.mlir b/mlir/test/Dialect/LLVMIR/inlining-alias-scopes.mlir
index 0b8b60e963bb01..ce414cc94dc3ca 100644
--- a/mlir/test/Dialect/LLVMIR/inlining-alias-scopes.mlir
+++ b/mlir/test/Dialect/LLVMIR/inlining-alias-scopes.mlir
@@ -296,6 +296,39 @@ llvm.func @bar(%arg0: !llvm.ptr, %arg1: !llvm.ptr, %arg2: !llvm.ptr) {
llvm.func @random() -> i1
+llvm.func @region_branch(%arg0: !llvm.ptr {llvm.noalias}, %arg1: !llvm.ptr {llvm.noalias}) {
+ %0 = llvm.mlir.constant(5 : i64) : i32
+ test.region_if %arg0: !llvm.ptr -> !llvm.ptr then {
+ ^bb0(%arg2: !llvm.ptr):
+ test.region_if_yield %arg0 : !llvm.ptr
+ } else {
+ ^bb0(%arg2: !llvm.ptr):
+ test.region_if_yield %arg0 : !llvm.ptr
+ } join {
+ ^bb0(%arg2: !llvm.ptr):
+ llvm.store %0, %arg2 : i32, !llvm.ptr
+ test.region_if_yield %arg0 : !llvm.ptr
+ }
+ llvm.return
+}
+
+// CHECK-LABEL: llvm.func @region_branch_inlining
+// CHECK: llvm.store
+// CHECK-SAME: alias_scopes = [#[[$ARG0_SCOPE]]]
+// CHECK-SAME: noalias_scopes = [#[[$ARG1_SCOPE]]]
+llvm.func @region_branch_inlining(%arg0: !llvm.ptr, %arg1: !llvm.ptr, %arg2: !llvm.ptr) {
+ llvm.call @region_branch(%arg0, %arg2) : (!llvm.ptr, !llvm.ptr) -> ()
+ llvm.return
+}
+
+// -----
+
+// CHECK-DAG: #[[DOMAIN:.*]] = #llvm.alias_scope_domain<{{.*}}>
+// CHECK-DAG: #[[$ARG0_SCOPE:.*]] = #llvm.alias_scope<id = {{.*}}, domain = #[[DOMAIN]]{{(,.*)?}}>
+// CHECK-DAG: #[[$ARG1_SCOPE:.*]] = #llvm.alias_scope<id = {{.*}}, domain = #[[DOMAIN]]{{(,.*)?}}>
+
+llvm.func @random() -> i1
+
llvm.func @block_arg(%arg0: !llvm.ptr {llvm.noalias}, %arg1: !llvm.ptr {llvm.noalias}) {
%0 = llvm.mlir.constant(5 : i64) : i32
%1 = llvm.mlir.constant(1 : i64) : i64
>From 8ea911eee6ab302532d4a9105cc627b46bbf3684 Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Wed, 14 Aug 2024 12:07:42 +0000
Subject: [PATCH 4/6] rename to SliceWalk.*
---
mlir/include/mlir/IR/{SliceSupport.h => SliceWalk.h} | 8 ++++----
.../Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp | 2 +-
mlir/lib/IR/CMakeLists.txt | 2 +-
mlir/lib/IR/{SliceSupport.cpp => SliceWalk.cpp} | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
rename mlir/include/mlir/IR/{SliceSupport.h => SliceWalk.h} (95%)
rename mlir/lib/IR/{SliceSupport.cpp => SliceWalk.cpp} (99%)
diff --git a/mlir/include/mlir/IR/SliceSupport.h b/mlir/include/mlir/IR/SliceWalk.h
similarity index 95%
rename from mlir/include/mlir/IR/SliceSupport.h
rename to mlir/include/mlir/IR/SliceWalk.h
index a28b24ffcb3911..d01736b2218047 100644
--- a/mlir/include/mlir/IR/SliceSupport.h
+++ b/mlir/include/mlir/IR/SliceWalk.h
@@ -1,4 +1,4 @@
-//===- SliceSupport.h - Helpers for performing IR slicing -------*- C++ -*-===//
+//===- SliceWalk.h - Helpers for performing IR slice walks ---*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -6,8 +6,8 @@
//
//===----------------------------------------------------------------------===//
-#ifndef MLIR_IR_SLICESUPPORT_H
-#define MLIR_IR_SLICESUPPORT_H
+#ifndef MLIR_IR_SLICEWALK_IR
+#define MLIR_IR_SLICEWALK_IR
#include "mlir/IR/ValueRange.h"
@@ -96,4 +96,4 @@ std::optional<SmallVector<Value>> getControlFlowPredecessors(Value value);
} // namespace mlir
-#endif // MLIR_IR_SLICESUPPORT_H
+#endif // MLIR_IR_SLICEWALK_IR
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
index cba9460bf92569..c9b65fa942bb99 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
@@ -14,7 +14,7 @@
#include "mlir/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.h"
#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
#include "mlir/IR/Matchers.h"
-#include "mlir/IR/SliceSupport.h"
+#include "mlir/IR/SliceWalk.h"
#include "mlir/Interfaces/DataLayoutInterfaces.h"
#include "mlir/Transforms/InliningUtils.h"
#include "llvm/ADT/ScopeExit.h"
diff --git a/mlir/lib/IR/CMakeLists.txt b/mlir/lib/IR/CMakeLists.txt
index bfff29abc824a0..d42c89c903b728 100644
--- a/mlir/lib/IR/CMakeLists.txt
+++ b/mlir/lib/IR/CMakeLists.txt
@@ -32,7 +32,7 @@ add_mlir_library(MLIRIR
PatternMatch.cpp
Region.cpp
RegionKindInterface.cpp
- SliceSupport.cpp
+ SliceWalk.cpp
SymbolTable.cpp
TensorEncoding.cpp
Types.cpp
diff --git a/mlir/lib/IR/SliceSupport.cpp b/mlir/lib/IR/SliceWalk.cpp
similarity index 99%
rename from mlir/lib/IR/SliceSupport.cpp
rename to mlir/lib/IR/SliceWalk.cpp
index 0c4d4e15fe83cf..5739d53f0715f5 100644
--- a/mlir/lib/IR/SliceSupport.cpp
+++ b/mlir/lib/IR/SliceWalk.cpp
@@ -1,4 +1,4 @@
-#include "mlir/IR/SliceSupport.h"
+#include "mlir/IR/SliceWalk.h"
#include "mlir/Interfaces/ControlFlowInterfaces.h"
using namespace mlir;
>From e216f6cc6afc0a74c1de3e99274c53a290435d9f Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Wed, 14 Aug 2024 13:25:19 +0000
Subject: [PATCH 5/6] address review comments
---
mlir/include/mlir/IR/SliceWalk.h | 7 +++----
.../Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp | 4 ++--
mlir/lib/IR/SliceWalk.cpp | 9 ++++-----
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/mlir/include/mlir/IR/SliceWalk.h b/mlir/include/mlir/IR/SliceWalk.h
index d01736b2218047..40fedbba6546c6 100644
--- a/mlir/include/mlir/IR/SliceWalk.h
+++ b/mlir/include/mlir/IR/SliceWalk.h
@@ -45,8 +45,7 @@ class WalkContinuation {
}
/// Creates a continuation that adds the user-specified `nextValues` to the
- /// work list and advances the walk. Unlike advance, this function does not
- /// add the control flow predecessor values to the work list.
+ /// work list and advances the walk.
static WalkContinuation advanceTo(mlir::ValueRange nextValues) {
return WalkContinuation(WalkAction::AdvanceTo, nextValues);
}
@@ -90,8 +89,8 @@ WalkContinuation walkSlice(mlir::ValueRange rootValues,
/// Computes a vector of all control predecessors of `value`. Relies on
/// RegionBranchOpInterface and BranchOpInterface to determine predecessors.
-/// Returns nullopt if value has no predecessors or when the relevant operations
-/// are missing the interface implementations.
+/// Returns nullopt if `value` has no predecessors or when the relevant
+/// operations are missing the interface implementations.
std::optional<SmallVector<Value>> getControlFlowPredecessors(Value value);
} // namespace mlir
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
index c9b65fa942bb99..0b64627ba4e8dc 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
@@ -253,8 +253,8 @@ getUnderlyingObjectSet(Value pointerValue) {
}
// If this place is reached, `val` is a block argument that is not
- // understood. Therfore, we conservatively interrupt.
- // Note: Dealing with the function arguments is not necessary, as the slice
+ // understood. Therefore, we conservatively interrupt.
+ // Note: Dealing with function arguments is not necessary, as the slice
// would have to go through an SSACopyOp first.
return WalkContinuation::interrupt();
});
diff --git a/mlir/lib/IR/SliceWalk.cpp b/mlir/lib/IR/SliceWalk.cpp
index 5739d53f0715f5..9244b286f9a2c5 100644
--- a/mlir/lib/IR/SliceWalk.cpp
+++ b/mlir/lib/IR/SliceWalk.cpp
@@ -78,9 +78,8 @@ getRegionPredecessorOperands(RegionBranchOpInterface regionOp,
return predecessorOperands;
}
-/// Returns the predecessor branch operands that match `blockArg`. Returns a
-/// nullopt when some of the predecessor terminators do not implement the
-/// BranchOpInterface.
+/// Returns the predecessor branch operands that match `blockArg`, or nullopt if
+/// some of the predecessor terminators do not implement the BranchOpInterface.
static std::optional<SmallVector<Value>>
getBlockPredecessorOperands(BlockArgument blockArg) {
Block *block = blockArg.getOwner();
@@ -130,8 +129,8 @@ mlir::getControlFlowPredecessors(Value value) {
regionBranchOp, region, blockArg.getArgNumber());
return predecessorOperands;
}
- // Unclear how to deal with this operation, conservatively return a
- // failure.
+ // If the interface is not implemented, there are no control flow
+ // predecessors to work with.
return std::nullopt;
}
>From 7521cd1b6ab146af57af54413b4220f54a7e7d8b Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Wed, 14 Aug 2024 13:41:36 +0000
Subject: [PATCH 6/6] move to analysis directory
---
mlir/include/mlir/{IR => Analysis}/SliceWalk.h | 6 +++---
mlir/lib/Analysis/CMakeLists.txt | 1 +
mlir/lib/{IR => Analysis}/SliceWalk.cpp | 2 +-
mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp | 2 +-
mlir/lib/IR/CMakeLists.txt | 1 -
5 files changed, 6 insertions(+), 6 deletions(-)
rename mlir/include/mlir/{IR => Analysis}/SliceWalk.h (97%)
rename mlir/lib/{IR => Analysis}/SliceWalk.cpp (99%)
diff --git a/mlir/include/mlir/IR/SliceWalk.h b/mlir/include/mlir/Analysis/SliceWalk.h
similarity index 97%
rename from mlir/include/mlir/IR/SliceWalk.h
rename to mlir/include/mlir/Analysis/SliceWalk.h
index 40fedbba6546c6..202b8199de319b 100644
--- a/mlir/include/mlir/IR/SliceWalk.h
+++ b/mlir/include/mlir/Analysis/SliceWalk.h
@@ -6,8 +6,8 @@
//
//===----------------------------------------------------------------------===//
-#ifndef MLIR_IR_SLICEWALK_IR
-#define MLIR_IR_SLICEWALK_IR
+#ifndef MLIR_ANALYSIS_SLICEWALK_IR
+#define MLIR_ANALYSIS_SLICEWALK_IR
#include "mlir/IR/ValueRange.h"
@@ -95,4 +95,4 @@ std::optional<SmallVector<Value>> getControlFlowPredecessors(Value value);
} // namespace mlir
-#endif // MLIR_IR_SLICEWALK_IR
+#endif // MLIR_ANALYSIS_SLICEWALK_IR
diff --git a/mlir/lib/Analysis/CMakeLists.txt b/mlir/lib/Analysis/CMakeLists.txt
index 38d8415d81c72d..609cb34309829e 100644
--- a/mlir/lib/Analysis/CMakeLists.txt
+++ b/mlir/lib/Analysis/CMakeLists.txt
@@ -29,6 +29,7 @@ add_mlir_library(MLIRAnalysis
Liveness.cpp
CFGLoopInfo.cpp
SliceAnalysis.cpp
+ SliceWalk.cpp
TopologicalSortUtils.cpp
AliasAnalysis/LocalAliasAnalysis.cpp
diff --git a/mlir/lib/IR/SliceWalk.cpp b/mlir/lib/Analysis/SliceWalk.cpp
similarity index 99%
rename from mlir/lib/IR/SliceWalk.cpp
rename to mlir/lib/Analysis/SliceWalk.cpp
index 9244b286f9a2c5..9d770639dc53ca 100644
--- a/mlir/lib/IR/SliceWalk.cpp
+++ b/mlir/lib/Analysis/SliceWalk.cpp
@@ -1,4 +1,4 @@
-#include "mlir/IR/SliceWalk.h"
+#include "mlir/Analysis/SliceWalk.h"
#include "mlir/Interfaces/ControlFlowInterfaces.h"
using namespace mlir;
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
index 0b64627ba4e8dc..504f63b48c9433 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.cpp
@@ -12,9 +12,9 @@
//===----------------------------------------------------------------------===//
#include "mlir/Dialect/LLVMIR/Transforms/InlinerInterfaceImpl.h"
+#include "mlir/Analysis/SliceWalk.h"
#include "mlir/Dialect/LLVMIR/LLVMDialect.h"
#include "mlir/IR/Matchers.h"
-#include "mlir/IR/SliceWalk.h"
#include "mlir/Interfaces/DataLayoutInterfaces.h"
#include "mlir/Transforms/InliningUtils.h"
#include "llvm/ADT/ScopeExit.h"
diff --git a/mlir/lib/IR/CMakeLists.txt b/mlir/lib/IR/CMakeLists.txt
index d42c89c903b728..c38ce6c058a006 100644
--- a/mlir/lib/IR/CMakeLists.txt
+++ b/mlir/lib/IR/CMakeLists.txt
@@ -32,7 +32,6 @@ add_mlir_library(MLIRIR
PatternMatch.cpp
Region.cpp
RegionKindInterface.cpp
- SliceWalk.cpp
SymbolTable.cpp
TensorEncoding.cpp
Types.cpp
More information about the llvm-branch-commits
mailing list