Fix remaining build failures with GCC 8.3

[RoboTux]

When compiling for GCC 8.x (< 8.4), SFINAE is disabled for iterator_range constructor causing ambiguous resolution to construct an OperandRange from a MutableOperatorRange, even in the presence of a static_cast. This adds an explicit conversion method to lift the ambiguity.

Tested with a full MLIR build with GCC 8.3.

[llvmbot]

@llvm/pr-subscribers-mlir-cf
@llvm/pr-subscribers-mlir-core
@llvm/pr-subscribers-mlir-bufferization
@llvm/pr-subscribers-mlir-linalg

@llvm/pr-subscribers-mlir

Author: Thomas Preud'homme (RoboTux)

Changes

When compiling for GCC 8.x (< 8.4), SFINAE is disabled for iterator_range constructor causing ambiguous resolution to construct an OperandRange from a MutableOperatorRange, even in the presence of a static_cast<OperatorRange>. This adds an explicit conversion method to lift the ambiguity.

Tested with a full MLIR build with GCC 8.3.

---

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

6 Files Affected:

• (modified) mlir/include/mlir/IR/ValueRange.h (+3)
• (modified) mlir/lib/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation.cpp (+2-2)
• (modified) mlir/lib/Dialect/ControlFlow/Transforms/BufferDeallocationOpInterfaceImpl.cpp (+1-1)
• (modified) mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp (+1-1)
• (modified) mlir/lib/IR/OperationSupport.cpp (+6-1)
• (modified) mlir/lib/Transforms/Utils/CFGToSCF.cpp (+1-2)

diff --git a/mlir/include/mlir/IR/ValueRange.h b/mlir/include/mlir/IR/ValueRange.h
index 51262e2d78716e..4b421c08d8418e 100644
--- a/mlir/include/mlir/IR/ValueRange.h
+++ b/mlir/include/mlir/IR/ValueRange.h
@@ -155,6 +155,9 @@ class MutableOperandRange {
   /// Returns if the current range is empty.
   bool empty() const { return size() == 0; }
 
+  /// Explicit conversion to an OperandRange.
+  OperandRange getAsOperandRange() const;
+
   /// Allow implicit conversion to an OperandRange.
   operator OperandRange() const;
 
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation.cpp b/mlir/lib/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation.cpp
index 0a7494c86d86fc..c9fd110d48d9a9 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation.cpp
@@ -956,13 +956,13 @@ BufferDeallocation::handleInterface(RegionBranchTerminatorOpInterface op) {
 
   SmallVector<Value> updatedOwnerships;
   auto result = deallocation_impl::insertDeallocOpForReturnLike(
-      state, op, OperandRange(operands), updatedOwnerships);
+      state, op, operands.getAsOperandRange(), updatedOwnerships);
   if (failed(result) || !*result)
     return result;
 
   // Add an additional operand for every MemRef for the ownership indicator.
   if (!funcWithoutDynamicOwnership) {
-    SmallVector<Value> newOperands{OperandRange(operands)};
+    SmallVector<Value> newOperands{operands.getAsOperandRange()};
     newOperands.append(updatedOwnerships.begin(), updatedOwnerships.end());
     operands.assign(newOperands);
   }
diff --git a/mlir/lib/Dialect/ControlFlow/Transforms/BufferDeallocationOpInterfaceImpl.cpp b/mlir/lib/Dialect/ControlFlow/Transforms/BufferDeallocationOpInterfaceImpl.cpp
index 9423af2542690d..0dc357c2298fad 100644
--- a/mlir/lib/Dialect/ControlFlow/Transforms/BufferDeallocationOpInterfaceImpl.cpp
+++ b/mlir/lib/Dialect/ControlFlow/Transforms/BufferDeallocationOpInterfaceImpl.cpp
@@ -84,7 +84,7 @@ struct CondBranchOpInterface
             DenseMap<Value, Value> &mapping) -> DeallocOp {
       SmallVector<Value> toRetain;
       state.getMemrefsToRetain(condBr->getBlock(), target,
-                               OperandRange(destOperands), toRetain);
+                               destOperands.getAsOperandRange(), toRetain);
       SmallVector<Value> adaptedConditions(
           llvm::map_range(conditions, conditionModifier));
       auto deallocOp = builder.create<bufferization::DeallocOp>(
diff --git a/mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp b/mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp
index a85532b2f755a4..223d728b0b27d5 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp
@@ -149,7 +149,7 @@ static LinalgOp fuse(OpBuilder &b, LinalgOp producer,
   SmallVector<Type, 4> resultTypes;
   resultTypes.reserve(producer->getNumResults());
   int64_t firstInitOperandIdx =
-      static_cast<OperandRange>(producerDpsInits).getBeginOperandIndex();
+      producerDpsInits.getAsOperandRange().getBeginOperandIndex();
   for (int64_t i = 0, e = producer->getNumResults(); i < e; ++i) {
     resultTypes.push_back(clonedShapes[firstInitOperandIdx + i].getType());
   }
diff --git a/mlir/lib/IR/OperationSupport.cpp b/mlir/lib/IR/OperationSupport.cpp
index a168fe30ba8a48..a72ccb9ca490c6 100644
--- a/mlir/lib/IR/OperationSupport.cpp
+++ b/mlir/lib/IR/OperationSupport.cpp
@@ -497,9 +497,14 @@ void MutableOperandRange::clear() {
   }
 }
 
+/// Explicit conversion to an OperandRange.
+OperandRange MutableOperandRange::getAsOperandRange() const {
+  return owner->getOperands().slice(start, length);
+}
+
 /// Allow implicit conversion to an OperandRange.
 MutableOperandRange::operator OperandRange() const {
-  return owner->getOperands().slice(start, length);
+  return getAsOperandRange();
 }
 
 MutableOperandRange::operator MutableArrayRef<OpOperand>() const {
diff --git a/mlir/lib/Transforms/Utils/CFGToSCF.cpp b/mlir/lib/Transforms/Utils/CFGToSCF.cpp
index f2998b4047e201..eefdf1d4e393af 100644
--- a/mlir/lib/Transforms/Utils/CFGToSCF.cpp
+++ b/mlir/lib/Transforms/Utils/CFGToSCF.cpp
@@ -1183,8 +1183,7 @@ static FailureOr<SmallVector<Block *>> transformToStructuredCFBranches(
     auto builder = OpBuilder::atBlockTerminator(user->getBlock());
     LogicalResult result = interface.createStructuredBranchRegionTerminatorOp(
         user->getLoc(), builder, structuredCondOp, user,
-        static_cast<OperandRange>(
-            getMutableSuccessorOperands(user->getBlock(), 0)));
+        getMutableSuccessorOperands(user->getBlock(), 0).getAsOperandRange());
     if (failed(result))
       return failure();
     user->erase();

[dwblaikie]

Guess there's some conversations happening about which versions we should support (might be worth crosslinking those conversations to these changes if it hasn't been done already) - so maybe give it a few days to see if other folks object to this support being added - but the specific change seems ok to me.

[RoboTux]

Guess there's some conversations happening about which versions we should support (might be worth crosslinking those conversations to these changes if it hasn't been done already) - so maybe give it a few days to see if other folks object to this support being added - but the specific change seems ok to me.

Since there hasn't been any comment I'm going to assume people are fine with this and merge it.