[llvm] [mlir] Make MLIR Value more consistent in terms of `const` "correctness" (NFC) (PR #72765)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Nov 18 14:33:12 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-llvm
Author: Mehdi Amini (joker-eph)
<details>
<summary>Changes</summary>
MLIR can't really be const-correct (it would need a `ConstValue` class alongside the `Value` class really, like `ArrayRef` and `MutableArrayRef`). This is however making is more consistent: method that are directly modifying the Value shouldn't be marked const.
---
Full diff: https://github.com/llvm/llvm-project/pull/72765.diff
18 Files Affected:
- (modified) llvm/include/llvm/ADT/ArrayRef.h (+3-2)
- (modified) mlir/docs/Tutorials/Toy/Ch-4.md (+1-1)
- (modified) mlir/examples/toy/Ch4/mlir/Dialect.cpp (+1-1)
- (modified) mlir/examples/toy/Ch5/mlir/Dialect.cpp (+1-1)
- (modified) mlir/examples/toy/Ch6/mlir/Dialect.cpp (+1-1)
- (modified) mlir/examples/toy/Ch7/mlir/Dialect.cpp (+1-1)
- (modified) mlir/include/mlir/IR/Value.h (+12-13)
- (modified) mlir/include/mlir/Transforms/InliningUtils.h (+2-2)
- (modified) mlir/lib/Conversion/ControlFlowToSPIRV/ControlFlowToSPIRV.cpp (+1-1)
- (modified) mlir/lib/Dialect/Func/Extensions/InlinerExtension.cpp (+1-1)
- (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp (+1-1)
- (modified) mlir/lib/Dialect/Linalg/IR/LinalgDialect.cpp (+1-1)
- (modified) mlir/lib/Dialect/SCF/IR/SCF.cpp (+1-1)
- (modified) mlir/lib/Dialect/SPIRV/IR/SPIRVDialect.cpp (+1-1)
- (modified) mlir/lib/IR/AsmPrinter.cpp (+7-6)
- (modified) mlir/lib/IR/Value.cpp (+3-4)
- (modified) mlir/lib/Transforms/Utils/InliningUtils.cpp (+5-4)
- (modified) mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp (+1-1)
``````````diff
diff --git a/llvm/include/llvm/ADT/ArrayRef.h b/llvm/include/llvm/ADT/ArrayRef.h
index 713f463f65edf17..65a2d523a741cac 100644
--- a/llvm/include/llvm/ADT/ArrayRef.h
+++ b/llvm/include/llvm/ADT/ArrayRef.h
@@ -335,8 +335,9 @@ namespace llvm {
MutableArrayRef(T *begin, T *end) : ArrayRef<T>(begin, end) {}
/// Construct a MutableArrayRef from a SmallVector.
- /*implicit*/ MutableArrayRef(SmallVectorImpl<T> &Vec)
- : ArrayRef<T>(Vec) {}
+ template <typename U>
+ /*implicit*/ MutableArrayRef(const SmallVectorTemplateCommon<T, U> &Vec)
+ : ArrayRef<T>(Vec) {}
/// Construct a MutableArrayRef from a std::vector.
/*implicit*/ MutableArrayRef(std::vector<T> &Vec)
diff --git a/mlir/docs/Tutorials/Toy/Ch-4.md b/mlir/docs/Tutorials/Toy/Ch-4.md
index b9369fec1909682..ae10dc4a0113d76 100644
--- a/mlir/docs/Tutorials/Toy/Ch-4.md
+++ b/mlir/docs/Tutorials/Toy/Ch-4.md
@@ -91,7 +91,7 @@ struct ToyInlinerInterface : public DialectInlinerInterface {
/// previously returned by the call operation with the operands of the
/// return.
void handleTerminator(Operation *op,
- ArrayRef<Value> valuesToRepl) const final {
+ MutableArrayRef<Value> valuesToRepl) const final {
// Only "toy.return" needs to be handled here.
auto returnOp = cast<ReturnOp>(op);
diff --git a/mlir/examples/toy/Ch4/mlir/Dialect.cpp b/mlir/examples/toy/Ch4/mlir/Dialect.cpp
index cc0ea5c4a63750b..4b83abb7f6f3b55 100644
--- a/mlir/examples/toy/Ch4/mlir/Dialect.cpp
+++ b/mlir/examples/toy/Ch4/mlir/Dialect.cpp
@@ -75,7 +75,7 @@ struct ToyInlinerInterface : public DialectInlinerInterface {
/// Handle the given inlined terminator(toy.return) by replacing it with a new
/// operation as necessary.
void handleTerminator(Operation *op,
- ArrayRef<Value> valuesToRepl) const final {
+ MutableArrayRef<Value> valuesToRepl) const final {
// Only "toy.return" needs to be handled here.
auto returnOp = cast<ReturnOp>(op);
diff --git a/mlir/examples/toy/Ch5/mlir/Dialect.cpp b/mlir/examples/toy/Ch5/mlir/Dialect.cpp
index 74adfeb64cce5ee..12e398b5584a527 100644
--- a/mlir/examples/toy/Ch5/mlir/Dialect.cpp
+++ b/mlir/examples/toy/Ch5/mlir/Dialect.cpp
@@ -75,7 +75,7 @@ struct ToyInlinerInterface : public DialectInlinerInterface {
/// Handle the given inlined terminator(toy.return) by replacing it with a new
/// operation as necessary.
void handleTerminator(Operation *op,
- ArrayRef<Value> valuesToRepl) const final {
+ MutableArrayRef<Value> valuesToRepl) const final {
// Only "toy.return" needs to be handled here.
auto returnOp = cast<ReturnOp>(op);
diff --git a/mlir/examples/toy/Ch6/mlir/Dialect.cpp b/mlir/examples/toy/Ch6/mlir/Dialect.cpp
index 74adfeb64cce5ee..12e398b5584a527 100644
--- a/mlir/examples/toy/Ch6/mlir/Dialect.cpp
+++ b/mlir/examples/toy/Ch6/mlir/Dialect.cpp
@@ -75,7 +75,7 @@ struct ToyInlinerInterface : public DialectInlinerInterface {
/// Handle the given inlined terminator(toy.return) by replacing it with a new
/// operation as necessary.
void handleTerminator(Operation *op,
- ArrayRef<Value> valuesToRepl) const final {
+ MutableArrayRef<Value> valuesToRepl) const final {
// Only "toy.return" needs to be handled here.
auto returnOp = cast<ReturnOp>(op);
diff --git a/mlir/examples/toy/Ch7/mlir/Dialect.cpp b/mlir/examples/toy/Ch7/mlir/Dialect.cpp
index f17173f007645f1..388b4132a99fb76 100644
--- a/mlir/examples/toy/Ch7/mlir/Dialect.cpp
+++ b/mlir/examples/toy/Ch7/mlir/Dialect.cpp
@@ -81,7 +81,7 @@ struct ToyInlinerInterface : public DialectInlinerInterface {
/// Handle the given inlined terminator(toy.return) by replacing it with a new
/// operation as necessary.
void handleTerminator(Operation *op,
- ArrayRef<Value> valuesToRepl) const final {
+ MutableArrayRef<Value> valuesToRepl) const final {
// Only "toy.return" needs to be handled here.
auto returnOp = cast<ReturnOp>(op);
diff --git a/mlir/include/mlir/IR/Value.h b/mlir/include/mlir/IR/Value.h
index dbcc10d4f4df80e..ae63eb29e4872f6 100644
--- a/mlir/include/mlir/IR/Value.h
+++ b/mlir/include/mlir/IR/Value.h
@@ -158,26 +158,25 @@ class Value {
//===--------------------------------------------------------------------===//
/// Drop all uses of this object from their respective owners.
- void dropAllUses() const { return impl->dropAllUses(); }
+ void dropAllUses() { return impl->dropAllUses(); }
/// Replace all uses of 'this' value with the new value, updating anything in
/// the IR that uses 'this' to use the other value instead. When this returns
/// there are zero uses of 'this'.
- void replaceAllUsesWith(Value newValue) const {
+ void replaceAllUsesWith(Value newValue) {
impl->replaceAllUsesWith(newValue);
}
/// Replace all uses of 'this' value with 'newValue', updating anything in the
/// IR that uses 'this' to use the other value instead except if the user is
/// listed in 'exceptions' .
- void
- replaceAllUsesExcept(Value newValue,
- const SmallPtrSetImpl<Operation *> &exceptions) const;
+ void replaceAllUsesExcept(Value newValue,
+ const SmallPtrSetImpl<Operation *> &exceptions);
/// Replace all uses of 'this' value with 'newValue', updating anything in the
/// IR that uses 'this' to use the other value instead except if the user is
/// 'exceptedUser'.
- void replaceAllUsesExcept(Value newValue, Operation *exceptedUser) const;
+ void replaceAllUsesExcept(Value newValue, Operation *exceptedUser);
/// Replace all uses of 'this' value with 'newValue' if the given callback
/// returns true.
@@ -185,7 +184,7 @@ class Value {
function_ref<bool(OpOperand &)> shouldReplace);
/// Returns true if the value is used outside of the given block.
- bool isUsedOutsideOfBlock(Block *block);
+ bool isUsedOutsideOfBlock(Block *block) const;
/// Shuffle the use list order according to the provided indices. It is
/// responsibility of the caller to make sure that the indices map the current
@@ -224,14 +223,14 @@ class Value {
//===--------------------------------------------------------------------===//
// Utilities
- void print(raw_ostream &os);
- void print(raw_ostream &os, const OpPrintingFlags &flags);
- void print(raw_ostream &os, AsmState &state);
- void dump();
+ void print(raw_ostream &os) const;
+ void print(raw_ostream &os, const OpPrintingFlags &flags) const;
+ void print(raw_ostream &os, AsmState &state) const;
+ void dump() const;
/// Print this value as if it were an operand.
- void printAsOperand(raw_ostream &os, AsmState &state);
- void printAsOperand(raw_ostream &os, const OpPrintingFlags &flags);
+ void printAsOperand(raw_ostream &os, AsmState &state) const;
+ void printAsOperand(raw_ostream &os, const OpPrintingFlags &flags) const;
/// Methods for supporting PointerLikeTypeTraits.
void *getAsOpaquePointer() const { return impl; }
diff --git a/mlir/include/mlir/Transforms/InliningUtils.h b/mlir/include/mlir/Transforms/InliningUtils.h
index a602c4e208f1801..7bb58674de3cc6c 100644
--- a/mlir/include/mlir/Transforms/InliningUtils.h
+++ b/mlir/include/mlir/Transforms/InliningUtils.h
@@ -116,7 +116,7 @@ class DialectInlinerInterface
/// operation). The given 'op' will be removed by the caller, after this
/// function has been called.
virtual void handleTerminator(Operation *op,
- ArrayRef<Value> valuesToReplace) const {
+ MutableArrayRef<Value> valuesToReplace) const {
llvm_unreachable(
"must implement handleTerminator in the case of one inlined block");
}
@@ -212,7 +212,7 @@ class InlinerInterface
virtual void handleTerminator(Operation *op, Block *newDest) const;
virtual void handleTerminator(Operation *op,
- ArrayRef<Value> valuesToRepl) const;
+ MutableArrayRef<Value> valuesToRepl) const;
virtual Value handleArgument(OpBuilder &builder, Operation *call,
Operation *callable, Value argument,
diff --git a/mlir/lib/Conversion/ControlFlowToSPIRV/ControlFlowToSPIRV.cpp b/mlir/lib/Conversion/ControlFlowToSPIRV/ControlFlowToSPIRV.cpp
index 8907756d33845a0..995eecfc3e9c36e 100644
--- a/mlir/lib/Conversion/ControlFlowToSPIRV/ControlFlowToSPIRV.cpp
+++ b/mlir/lib/Conversion/ControlFlowToSPIRV/ControlFlowToSPIRV.cpp
@@ -34,7 +34,7 @@ static LogicalResult legalizeBlockArguments(Block &block, Operation *op,
const TypeConverter &converter) {
auto builder = OpBuilder::atBlockBegin(&block);
for (unsigned i = 0; i < block.getNumArguments(); ++i) {
- const auto arg = block.getArgument(i);
+ BlockArgument arg = block.getArgument(i);
if (converter.isLegal(arg.getType()))
continue;
Type ty = arg.getType();
diff --git a/mlir/lib/Dialect/Func/Extensions/InlinerExtension.cpp b/mlir/lib/Dialect/Func/Extensions/InlinerExtension.cpp
index 125cc3ca649fbb3..23f23e922cc35f5 100644
--- a/mlir/lib/Dialect/Func/Extensions/InlinerExtension.cpp
+++ b/mlir/lib/Dialect/Func/Extensions/InlinerExtension.cpp
@@ -64,7 +64,7 @@ struct FuncInlinerInterface : public DialectInlinerInterface {
/// Handle the given inlined terminator by replacing it with a new operation
/// as necessary.
void handleTerminator(Operation *op,
- ArrayRef<Value> valuesToRepl) const final {
+ MutableArrayRef<Value> valuesToRepl) const final {
// Only return needs to be handled here.
auto returnOp = cast<ReturnOp>(op);
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp
index 6063abdba7b9a1f..57452a1438697bc 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMInlining.cpp
@@ -738,7 +738,7 @@ struct LLVMInlinerInterface : public DialectInlinerInterface {
/// operands of the return. This overload is called when the inlined region
/// only contains one block.
void handleTerminator(Operation *op,
- ArrayRef<Value> valuesToRepl) const final {
+ MutableArrayRef<Value> valuesToRepl) const final {
// Return will be the only terminator present.
auto returnOp = cast<LLVM::ReturnOp>(op);
diff --git a/mlir/lib/Dialect/Linalg/IR/LinalgDialect.cpp b/mlir/lib/Dialect/Linalg/IR/LinalgDialect.cpp
index 30196e207d4a0ad..c25f6398e7b60f8 100644
--- a/mlir/lib/Dialect/Linalg/IR/LinalgDialect.cpp
+++ b/mlir/lib/Dialect/Linalg/IR/LinalgDialect.cpp
@@ -54,7 +54,7 @@ struct LinalgInlinerInterface : public DialectInlinerInterface {
// Handle the given inlined terminator by replacing it with a new operation
// as necessary. Required when the region has only one block.
void handleTerminator(Operation *op,
- ArrayRef<Value> valuesToRepl) const final {}
+ MutableArrayRef<Value> valuesToRepl) const final {}
};
} // namespace
diff --git a/mlir/lib/Dialect/SCF/IR/SCF.cpp b/mlir/lib/Dialect/SCF/IR/SCF.cpp
index 646284f8a9db435..e8cb63239da51b7 100644
--- a/mlir/lib/Dialect/SCF/IR/SCF.cpp
+++ b/mlir/lib/Dialect/SCF/IR/SCF.cpp
@@ -51,7 +51,7 @@ struct SCFInlinerInterface : public DialectInlinerInterface {
// Handle the given inlined terminator by replacing it with a new operation
// as necessary. Required when the region has only one block.
void handleTerminator(Operation *op,
- ArrayRef<Value> valuesToRepl) const final {
+ MutableArrayRef<Value> valuesToRepl) const final {
auto retValOp = dyn_cast<scf::YieldOp>(op);
if (!retValOp)
return;
diff --git a/mlir/lib/Dialect/SPIRV/IR/SPIRVDialect.cpp b/mlir/lib/Dialect/SPIRV/IR/SPIRVDialect.cpp
index 2de849dc4465e37..8c6ac90014945b5 100644
--- a/mlir/lib/Dialect/SPIRV/IR/SPIRVDialect.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/SPIRVDialect.cpp
@@ -102,7 +102,7 @@ struct SPIRVInlinerInterface : public DialectInlinerInterface {
/// Handle the given inlined terminator by replacing it with a new operation
/// as necessary.
void handleTerminator(Operation *op,
- ArrayRef<Value> valuesToRepl) const final {
+ MutableArrayRef<Value> valuesToRepl) const final {
// Only spirv.ReturnValue needs to be handled here.
auto retValOp = dyn_cast<spirv::ReturnValueOp>(op);
if (!retValOp)
diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index 6a67fc707b4976b..4b76dcf7f8a9f7c 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -3781,8 +3781,8 @@ void IntegerSet::print(raw_ostream &os) const {
AsmPrinter::Impl(os, state.getImpl()).printIntegerSet(*this);
}
-void Value::print(raw_ostream &os) { print(os, OpPrintingFlags()); }
-void Value::print(raw_ostream &os, const OpPrintingFlags &flags) {
+void Value::print(raw_ostream &os) const { print(os, OpPrintingFlags()); }
+void Value::print(raw_ostream &os, const OpPrintingFlags &flags) const {
if (!impl) {
os << "<<NULL VALUE>>";
return;
@@ -3795,7 +3795,7 @@ void Value::print(raw_ostream &os, const OpPrintingFlags &flags) {
os << "<block argument> of type '" << arg.getType()
<< "' at index: " << arg.getArgNumber();
}
-void Value::print(raw_ostream &os, AsmState &state) {
+void Value::print(raw_ostream &os, AsmState &state) const {
if (!impl) {
os << "<<NULL VALUE>>";
return;
@@ -3810,12 +3810,12 @@ void Value::print(raw_ostream &os, AsmState &state) {
<< "' at index: " << arg.getArgNumber();
}
-void Value::dump() {
+void Value::dump() const {
print(llvm::errs());
llvm::errs() << "\n";
}
-void Value::printAsOperand(raw_ostream &os, AsmState &state) {
+void Value::printAsOperand(raw_ostream &os, AsmState &state) const {
// TODO: This doesn't necessarily capture all potential cases.
// Currently, region arguments can be shadowed when printing the main
// operation. If the IR hasn't been printed, this will produce the old SSA
@@ -3840,7 +3840,8 @@ static Operation *findParent(Operation *op, bool shouldUseLocalScope) {
return op;
}
-void Value::printAsOperand(raw_ostream &os, const OpPrintingFlags &flags) {
+void Value::printAsOperand(raw_ostream &os,
+ const OpPrintingFlags &flags) const {
Operation *op;
if (auto result = llvm::dyn_cast<OpResult>(*this)) {
op = result.getOwner();
diff --git a/mlir/lib/IR/Value.cpp b/mlir/lib/IR/Value.cpp
index 6b5195da5e47ba2..178765353cc1058 100644
--- a/mlir/lib/IR/Value.cpp
+++ b/mlir/lib/IR/Value.cpp
@@ -59,7 +59,7 @@ Block *Value::getParentBlock() {
/// the IR that uses 'this' to use the other value instead except if the user is
/// listed in 'exceptions' .
void Value::replaceAllUsesExcept(
- Value newValue, const SmallPtrSetImpl<Operation *> &exceptions) const {
+ Value newValue, const SmallPtrSetImpl<Operation *> &exceptions) {
for (OpOperand &use : llvm::make_early_inc_range(getUses())) {
if (exceptions.count(use.getOwner()) == 0)
use.set(newValue);
@@ -69,8 +69,7 @@ void Value::replaceAllUsesExcept(
/// Replace all uses of 'this' value with 'newValue', updating anything in the
/// IR that uses 'this' to use the other value instead except if the user is
/// 'exceptedUser'.
-void Value::replaceAllUsesExcept(Value newValue,
- Operation *exceptedUser) const {
+void Value::replaceAllUsesExcept(Value newValue, Operation *exceptedUser) {
for (OpOperand &use : llvm::make_early_inc_range(getUses())) {
if (use.getOwner() != exceptedUser)
use.set(newValue);
@@ -87,7 +86,7 @@ void Value::replaceUsesWithIf(Value newValue,
}
/// Returns true if the value is used outside of the given block.
-bool Value::isUsedOutsideOfBlock(Block *block) {
+bool Value::isUsedOutsideOfBlock(Block *block) const {
return llvm::any_of(getUsers(), [block](Operation *user) {
return user->getBlock() != block;
});
diff --git a/mlir/lib/Transforms/Utils/InliningUtils.cpp b/mlir/lib/Transforms/Utils/InliningUtils.cpp
index 00c5405079def0e..d856f037b201015 100644
--- a/mlir/lib/Transforms/Utils/InliningUtils.cpp
+++ b/mlir/lib/Transforms/Utils/InliningUtils.cpp
@@ -96,8 +96,8 @@ void InlinerInterface::handleTerminator(Operation *op, Block *newDest) const {
/// Handle the given inlined terminator by replacing it with a new operation
/// as necessary.
-void InlinerInterface::handleTerminator(Operation *op,
- ArrayRef<Value> valuesToRepl) const {
+void InlinerInterface::handleTerminator(
+ Operation *op, MutableArrayRef<Value> valuesToRepl) const {
auto *handler = getInterfaceFor(op);
assert(handler && "expected valid dialect handler");
handler->handleTerminator(op, valuesToRepl);
@@ -289,8 +289,9 @@ inlineRegionImpl(InlinerInterface &interface, Region *src, Block *inlineBlock,
firstBlockTerminator->getOperands());
// Have the interface handle the terminator of this block.
- interface.handleTerminator(firstBlockTerminator,
- llvm::to_vector<6>(resultsToReplace));
+ interface.handleTerminator(
+ firstBlockTerminator,
+ MutableArrayRef<Value>{llvm::to_vector<6>(resultsToReplace)});
firstBlockTerminator->erase();
// Merge the post insert block into the cloned entry block.
diff --git a/mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp b/mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp
index ab7d2486db9aec7..2c99e918f959d3a 100644
--- a/mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp
+++ b/mlir/test/lib/Dialect/Test/TestDialectInterfaces.cpp
@@ -315,7 +315,7 @@ struct TestInlinerInterface : public DialectInlinerInterface {
/// Handle the given inlined terminator by replacing it with a new operation
/// as necessary.
void handleTerminator(Operation *op,
- ArrayRef<Value> valuesToRepl) const final {
+ MutableArrayRef<Value> valuesToRepl) const final {
// Only handle "test.return" here.
auto returnOp = dyn_cast<TestReturnOp>(op);
if (!returnOp)
``````````
</details>
https://github.com/llvm/llvm-project/pull/72765
More information about the llvm-commits
mailing list