[mlir] Migrate away from ValueRange(std::nullopt) (NFC) #145210
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
ArrayRef has a constructor that accepts std::nullopt. This
constructor dates back to the days when we still had llvm::Optional.
Since the use of std::nullopt outside the context of std::optional is
kind of abuse and not intuitive to new comers, I would like to move
away from the constructor and eventually remove it.
One of the common uses of std::nullopt is in one of the constructors
for ValueRange. This patch takes care of the migration where we need
ValueRange() to facilitate perfect forwarding. Note that {} would be
ambiguous for perfecting forwarding to work.
kazutakahirata
requested review from
PeimingLiu,
aartbik,
antiagainst,
kuhar,
matthias-springer and
yinying-lisa-li
as code owners
llvmbot
added
mlir:spirv
mlir:affine
mlir:sparse
Member
|
@llvm/pr-subscribers-mlir-affine @llvm/pr-subscribers-mlir-scf Author: Kazu Hirata (kazutakahirata) ChangesArrayRef has a constructor that accepts std::nullopt. This Since the use of std::nullopt outside the context of std::optional is One of the common uses of std::nullopt is in one of the constructors Full diff: https://github.com/llvm/llvm-project/pull/145210.diff 8 Files Affected:
diff --git a/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp b/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp
index 3d0804fd11b6b..cf416e140c85f 100644
--- a/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp
+++ b/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp
@@ -79,7 +79,7 @@ struct AssertOpLowering : public ConvertOpToLLVMPattern<cf::AssertOp> {
abortFunc = rewriter.create<LLVM::LLVMFuncOp>(rewriter.getUnknownLoc(),
"abort", abortFuncTy);
}
- rewriter.create<LLVM::CallOp>(loc, abortFunc, std::nullopt);
+ rewriter.create<LLVM::CallOp>(loc, abortFunc, ValueRange());
rewriter.create<LLVM::UnreachableOp>(loc);
} else {
rewriter.create<LLVM::BrOp>(loc, ValueRange(), continuationBlock);
diff --git a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
index e8294a5234c4f..4b7b2cc224ce2 100644
--- a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
+++ b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
@@ -1616,8 +1616,8 @@ struct MemRefReshapeOpLowering
// Hook up the cond exit to the remainder.
rewriter.setInsertionPointToEnd(condBlock);
- rewriter.create<LLVM::CondBrOp>(loc, pred, bodyBlock, std::nullopt,
- remainder, std::nullopt);
+ rewriter.create<LLVM::CondBrOp>(loc, pred, bodyBlock, ValueRange(),
+ remainder, ValueRange());
// Reset position to beginning of new remainder block.
rewriter.setInsertionPointToStart(remainder);
diff --git a/mlir/lib/Conversion/MemRefToSPIRV/MemRefToSPIRV.cpp b/mlir/lib/Conversion/MemRefToSPIRV/MemRefToSPIRV.cpp
index ff5b762a969d8..f8867c65fe7d9 100644
--- a/mlir/lib/Conversion/MemRefToSPIRV/MemRefToSPIRV.cpp
+++ b/mlir/lib/Conversion/MemRefToSPIRV/MemRefToSPIRV.cpp
@@ -928,7 +928,7 @@ LogicalResult ReinterpretCastPattern::matchAndRewrite(
}();
rewriter.replaceOpWithNewOp<spirv::InBoundsPtrAccessChainOp>(
- op, src, offsetValue, std::nullopt);
+ op, src, offsetValue, ValueRange());
return success();
}
diff --git a/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp b/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
index baac1b374b126..a59001de299fd 100644
--- a/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
+++ b/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
@@ -421,7 +421,7 @@ struct WhileOpConversion final : SCFToSPIRVPattern<scf::WhileOp> {
rewriter.setInsertionPointToEnd(&beforeBlock);
rewriter.replaceOpWithNewOp<spirv::BranchConditionalOp>(
- cond, conditionVal, &afterBlock, condArgs, &mergeBlock, std::nullopt);
+ cond, conditionVal, &afterBlock, condArgs, &mergeBlock, ValueRange());
// Convert the scf.yield op to a branch back to the header block.
rewriter.setInsertionPointToEnd(&afterBlock);
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index 3d09c6a9b2c24..48770d4f4ff7b 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -2742,7 +2742,7 @@ buildAffineLoopFromConstants(OpBuilder &builder, Location loc, int64_t lb,
int64_t ub, int64_t step,
AffineForOp::BodyBuilderFn bodyBuilderFn) {
return builder.create<AffineForOp>(loc, lb, ub, step,
- /*iterArgs=*/std::nullopt, bodyBuilderFn);
+ /*iterArgs=*/ValueRange(), bodyBuilderFn);
}
/// Creates an affine loop from the bounds that may or may not be constants.
@@ -2757,7 +2757,7 @@ buildAffineLoopFromValues(OpBuilder &builder, Location loc, Value lb, Value ub,
ubConst.value(), step, bodyBuilderFn);
return builder.create<AffineForOp>(loc, lb, builder.getDimIdentityMap(), ub,
builder.getDimIdentityMap(), step,
- /*iterArgs=*/std::nullopt, bodyBuilderFn);
+ /*iterArgs=*/ValueRange(), bodyBuilderFn);
}
void mlir::affine::buildAffineLoopNest(
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/LowerDeallocations.cpp b/mlir/lib/Dialect/Bufferization/Transforms/LowerDeallocations.cpp
index f51b125bda6e8..2a17ae4f6a249 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/LowerDeallocations.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/LowerDeallocations.cpp
@@ -460,14 +460,14 @@ func::FuncOp mlir::bufferization::buildDeallocationLibraryFunction(
Value toRetainSize = builder.create<memref::DimOp>(loc, toRetainMemref, c0);
builder.create<scf::ForOp>(
- loc, c0, toRetainSize, c1, std::nullopt,
+ loc, c0, toRetainSize, c1, ValueRange(),
[&](OpBuilder &builder, Location loc, Value i, ValueRange iterArgs) {
builder.create<memref::StoreOp>(loc, falseValue, retainCondsMemref, i);
builder.create<scf::YieldOp>(loc);
});
builder.create<scf::ForOp>(
- loc, c0, toDeallocSize, c1, std::nullopt,
+ loc, c0, toDeallocSize, c1, ValueRange(),
[&](OpBuilder &builder, Location loc, Value outerIter,
ValueRange iterArgs) {
Value toDealloc =
diff --git a/mlir/lib/Dialect/SCF/IR/SCF.cpp b/mlir/lib/Dialect/SCF/IR/SCF.cpp
index 5a0b8a058dd65..b3271462df274 100644
--- a/mlir/lib/Dialect/SCF/IR/SCF.cpp
+++ b/mlir/lib/Dialect/SCF/IR/SCF.cpp
@@ -3194,7 +3194,7 @@ struct MergeNestedParallelLoops : public OpRewritePattern<ParallelOp> {
auto newSteps = concatValues(op.getStep(), innerOp.getStep());
rewriter.replaceOpWithNewOp<ParallelOp>(op, newLowerBounds, newUpperBounds,
- newSteps, std::nullopt,
+ newSteps, ValueRange(),
bodyBuilder);
return success();
}
diff --git a/mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp b/mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
index dcb0237070885..093a35a3d4c94 100644
--- a/mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
+++ b/mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
@@ -1537,7 +1537,7 @@ struct OutRewriter : public OpRewritePattern<OutOp> {
// For each element in the source tensor, output the element.
rewriter.create<ForeachOp>(
- loc, src, std::nullopt,
+ loc, src, ValueRange(),
[&](OpBuilder &builder, Location loc, ValueRange dcvs, Value v,
ValueRange reduc) {
for (Dimension d = 0; d < dimRank; d++) {
|
Member
|
@llvm/pr-subscribers-mlir-spirv Author: Kazu Hirata (kazutakahirata) ChangesArrayRef has a constructor that accepts std::nullopt. This Since the use of std::nullopt outside the context of std::optional is One of the common uses of std::nullopt is in one of the constructors Full diff: https://github.com/llvm/llvm-project/pull/145210.diff 8 Files Affected:
diff --git a/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp b/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp
index 3d0804fd11b6b..cf416e140c85f 100644
--- a/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp
+++ b/mlir/lib/Conversion/ControlFlowToLLVM/ControlFlowToLLVM.cpp
@@ -79,7 +79,7 @@ struct AssertOpLowering : public ConvertOpToLLVMPattern<cf::AssertOp> {
abortFunc = rewriter.create<LLVM::LLVMFuncOp>(rewriter.getUnknownLoc(),
"abort", abortFuncTy);
}
- rewriter.create<LLVM::CallOp>(loc, abortFunc, std::nullopt);
+ rewriter.create<LLVM::CallOp>(loc, abortFunc, ValueRange());
rewriter.create<LLVM::UnreachableOp>(loc);
} else {
rewriter.create<LLVM::BrOp>(loc, ValueRange(), continuationBlock);
diff --git a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
index e8294a5234c4f..4b7b2cc224ce2 100644
--- a/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
+++ b/mlir/lib/Conversion/MemRefToLLVM/MemRefToLLVM.cpp
@@ -1616,8 +1616,8 @@ struct MemRefReshapeOpLowering
// Hook up the cond exit to the remainder.
rewriter.setInsertionPointToEnd(condBlock);
- rewriter.create<LLVM::CondBrOp>(loc, pred, bodyBlock, std::nullopt,
- remainder, std::nullopt);
+ rewriter.create<LLVM::CondBrOp>(loc, pred, bodyBlock, ValueRange(),
+ remainder, ValueRange());
// Reset position to beginning of new remainder block.
rewriter.setInsertionPointToStart(remainder);
diff --git a/mlir/lib/Conversion/MemRefToSPIRV/MemRefToSPIRV.cpp b/mlir/lib/Conversion/MemRefToSPIRV/MemRefToSPIRV.cpp
index ff5b762a969d8..f8867c65fe7d9 100644
--- a/mlir/lib/Conversion/MemRefToSPIRV/MemRefToSPIRV.cpp
+++ b/mlir/lib/Conversion/MemRefToSPIRV/MemRefToSPIRV.cpp
@@ -928,7 +928,7 @@ LogicalResult ReinterpretCastPattern::matchAndRewrite(
}();
rewriter.replaceOpWithNewOp<spirv::InBoundsPtrAccessChainOp>(
- op, src, offsetValue, std::nullopt);
+ op, src, offsetValue, ValueRange());
return success();
}
diff --git a/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp b/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
index baac1b374b126..a59001de299fd 100644
--- a/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
+++ b/mlir/lib/Conversion/SCFToSPIRV/SCFToSPIRV.cpp
@@ -421,7 +421,7 @@ struct WhileOpConversion final : SCFToSPIRVPattern<scf::WhileOp> {
rewriter.setInsertionPointToEnd(&beforeBlock);
rewriter.replaceOpWithNewOp<spirv::BranchConditionalOp>(
- cond, conditionVal, &afterBlock, condArgs, &mergeBlock, std::nullopt);
+ cond, conditionVal, &afterBlock, condArgs, &mergeBlock, ValueRange());
// Convert the scf.yield op to a branch back to the header block.
rewriter.setInsertionPointToEnd(&afterBlock);
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index 3d09c6a9b2c24..48770d4f4ff7b 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -2742,7 +2742,7 @@ buildAffineLoopFromConstants(OpBuilder &builder, Location loc, int64_t lb,
int64_t ub, int64_t step,
AffineForOp::BodyBuilderFn bodyBuilderFn) {
return builder.create<AffineForOp>(loc, lb, ub, step,
- /*iterArgs=*/std::nullopt, bodyBuilderFn);
+ /*iterArgs=*/ValueRange(), bodyBuilderFn);
}
/// Creates an affine loop from the bounds that may or may not be constants.
@@ -2757,7 +2757,7 @@ buildAffineLoopFromValues(OpBuilder &builder, Location loc, Value lb, Value ub,
ubConst.value(), step, bodyBuilderFn);
return builder.create<AffineForOp>(loc, lb, builder.getDimIdentityMap(), ub,
builder.getDimIdentityMap(), step,
- /*iterArgs=*/std::nullopt, bodyBuilderFn);
+ /*iterArgs=*/ValueRange(), bodyBuilderFn);
}
void mlir::affine::buildAffineLoopNest(
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/LowerDeallocations.cpp b/mlir/lib/Dialect/Bufferization/Transforms/LowerDeallocations.cpp
index f51b125bda6e8..2a17ae4f6a249 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/LowerDeallocations.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/LowerDeallocations.cpp
@@ -460,14 +460,14 @@ func::FuncOp mlir::bufferization::buildDeallocationLibraryFunction(
Value toRetainSize = builder.create<memref::DimOp>(loc, toRetainMemref, c0);
builder.create<scf::ForOp>(
- loc, c0, toRetainSize, c1, std::nullopt,
+ loc, c0, toRetainSize, c1, ValueRange(),
[&](OpBuilder &builder, Location loc, Value i, ValueRange iterArgs) {
builder.create<memref::StoreOp>(loc, falseValue, retainCondsMemref, i);
builder.create<scf::YieldOp>(loc);
});
builder.create<scf::ForOp>(
- loc, c0, toDeallocSize, c1, std::nullopt,
+ loc, c0, toDeallocSize, c1, ValueRange(),
[&](OpBuilder &builder, Location loc, Value outerIter,
ValueRange iterArgs) {
Value toDealloc =
diff --git a/mlir/lib/Dialect/SCF/IR/SCF.cpp b/mlir/lib/Dialect/SCF/IR/SCF.cpp
index 5a0b8a058dd65..b3271462df274 100644
--- a/mlir/lib/Dialect/SCF/IR/SCF.cpp
+++ b/mlir/lib/Dialect/SCF/IR/SCF.cpp
@@ -3194,7 +3194,7 @@ struct MergeNestedParallelLoops : public OpRewritePattern<ParallelOp> {
auto newSteps = concatValues(op.getStep(), innerOp.getStep());
rewriter.replaceOpWithNewOp<ParallelOp>(op, newLowerBounds, newUpperBounds,
- newSteps, std::nullopt,
+ newSteps, ValueRange(),
bodyBuilder);
return success();
}
diff --git a/mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp b/mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
index dcb0237070885..093a35a3d4c94 100644
--- a/mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
+++ b/mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
@@ -1537,7 +1537,7 @@ struct OutRewriter : public OpRewritePattern<OutOp> {
// For each element in the source tensor, output the element.
rewriter.create<ForeachOp>(
- loc, src, std::nullopt,
+ loc, src, ValueRange(),
[&](OpBuilder &builder, Location loc, ValueRange dcvs, Value v,
ValueRange reduc) {
for (Dimension d = 0; d < dimRank; d++) {
|
matthias-springer
approved these changes
Jun 22, 2025
kuhar
approved these changes
Jun 22, 2025
kazutakahirata
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
ArrayRef has a constructor that accepts std::nullopt. This
constructor dates back to the days when we still had llvm::Optional.
Since the use of std::nullopt outside the context of std::optional is
kind of abuse and not intuitive to new comers, I would like to move
away from the constructor and eventually remove it.
One of the common uses of std::nullopt is in one of the constructors
for ValueRange. This patch takes care of the migration where we need
ValueRange() to facilitate perfect forwarding. Note that {} would be
ambiguous for perfecting forwarding to work.