[mlir][IR] Mark getParentBlock() and getParentRegion() as const (NFC)

[CoTinker]

This PR marks Value::getParentBlock() and Value::getParentRegion() as const member functions. These methods do not modify the internal state of Value and are often used in read-only contexts. Marking them as const improves API clarity, allows greater flexibility in calling code.

[llvmbot]

@llvm/pr-subscribers-mlir-core

@llvm/pr-subscribers-mlir

Author: Longsheng Mou (CoTinker)

Changes

This PR marks Value::getParentBlock() and Value::getParentRegion() as const member functions. These methods do not modify the internal state of Value and are often used in read-only contexts. Marking them as const improves API clarity, allows greater flexibility in calling code.

---

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

2 Files Affected:

• (modified) mlir/include/mlir/IR/Value.h (+2-2)
• (modified) mlir/lib/IR/Value.cpp (+2-2)

diff --git a/mlir/include/mlir/IR/Value.h b/mlir/include/mlir/IR/Value.h
index 4d6d89fa69a07..e773d199c3c23 100644
--- a/mlir/include/mlir/IR/Value.h
+++ b/mlir/include/mlir/IR/Value.h
@@ -131,10 +131,10 @@ class Value {
   void setLoc(Location loc);
 
   /// Return the Region in which this Value is defined.
-  Region *getParentRegion();
+  Region *getParentRegion() const;
 
   /// Return the Block in which this Value is defined.
-  Block *getParentBlock();
+  Block *getParentBlock() const;
 
   //===--------------------------------------------------------------------===//
   // UseLists
diff --git a/mlir/lib/IR/Value.cpp b/mlir/lib/IR/Value.cpp
index fa550e4d5d5d0..2cffb962f8543 100644
--- a/mlir/lib/IR/Value.cpp
+++ b/mlir/lib/IR/Value.cpp
@@ -36,14 +36,14 @@ void Value::setLoc(Location loc) {
 }
 
 /// Return the Region in which this Value is defined.
-Region *Value::getParentRegion() {
+Region *Value::getParentRegion() const {
   if (auto *op = getDefiningOp())
     return op->getParentRegion();
   return llvm::cast<BlockArgument>(*this).getOwner()->getParent();
 }
 
 /// Return the Block in which this Value is defined.
-Block *Value::getParentBlock() {
+Block *Value::getParentBlock() const {
   if (Operation *op = getDefiningOp())
     return op->getBlock();
   return llvm::cast<BlockArgument>(*this).getOwner();

[matthias-springer]

I'm not quite sure how to interpret this document: https://mlir.llvm.org/docs/Rationale/UsageOfConst/
It suggests that functions like these should not be const. E.g.: you should never see a const method on Operation. Presumably that also extends to Value?

However, we have many const functions in Value.h today. So for consistency reasons, it makes sense to me to add the const keyword here.

[CoTinker]

Thanks for the explanation. I just read the document — it looks like they removed const from Operation.h, Block.h, and Region.h, but left it unchanged in Value.h. But I don't known why.

Edit:
It appears that the initial commit removed the const qualifiers in Value.h, but they were reintroduced in a follow-up commit.
The initial commit: 986310a#diff-aa926d0007a1d45c9173fc51fa2c54f263b9aad792758af53abd5b6ce2d59708. Should we remove the const in Value.h now? @lattner @jpienaar.

[CoTinker]

The reason behind this change is that Value::getParentOp() is already const, and since ValueComparator::operator() takes const Values, so I think it makes sense for getParentBlock() and getParentRegion() to be const as well.

llvm-project/mlir/lib/Dialect/Bufferization/IR/BufferDeallocationOpInterface.cpp

Line 222 in 19803d8

bool ValueComparator::operator()(const Value &lhs, const Value &rhs) const {

[lattner]

Yes, the same logic for removing const from Operation applies to Value as well. I'd be +1 on removing the const qualifiers from it.

[joker-eph]

The reason behind this change is that Value::getParentOp() is already const, and since ValueComparator::operator() takes const Values

I would think that ValueComparator can just const_cast the values in its implementation?

[CoTinker]

Yes, the same logic for removing const from Operation applies to Value as well. I'd be +1 on removing the const qualifiers from it.

Okay, thanks, I'll open a PR to remove the const.

[CoTinker]

The reason behind this change is that Value::getParentOp() is already const, and since ValueComparator::operator() takes const Values

I would think that ValueComparator can just const_cast the values in its implementation?

Fine, I'll fix it.