-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[mlir] Migrate away from TypeRange(std::nullopt) (NFC) #145246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[mlir] Migrate away from TypeRange(std::nullopt) (NFC) #145246
Conversation
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 uses of std::nullopt is in a the constructors for TypeRange. This patch takes care of the migration where we need TypeRange() to facilitate perfect forwarding. Note that {} would be ambiguous for perfecting forwarding to work.
@llvm/pr-subscribers-mlir-spirv @llvm/pr-subscribers-mlir 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 uses of std::nullopt is in a the constructors for Full diff: https://github.com/llvm/llvm-project/pull/145246.diff 3 Files Affected:
diff --git a/mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp b/mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp
index 1ac95ebcdc87f..84959d5ff8aba 100644
--- a/mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp
+++ b/mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp
@@ -245,7 +245,7 @@ void AffineLoopToGpuConverter::createLaunch(AffineForOp rootForOp,
Location terminatorLoc = terminator.getLoc();
terminator.erase();
builder.setInsertionPointToEnd(innermostForOp.getBody());
- builder.create<gpu::TerminatorOp>(terminatorLoc, std::nullopt);
+ builder.create<gpu::TerminatorOp>(terminatorLoc, TypeRange());
launchOp.getBody().front().getOperations().splice(
launchOp.getBody().front().begin(),
innermostForOp.getBody()->getOperations());
diff --git a/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp b/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
index 93979e0f73324..2e4561e7cc3ea 100644
--- a/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
+++ b/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
@@ -839,7 +839,7 @@ class FunctionCallPattern
ConversionPatternRewriter &rewriter) const override {
if (callOp.getNumResults() == 0) {
auto newOp = rewriter.replaceOpWithNewOp<LLVM::CallOp>(
- callOp, std::nullopt, adaptor.getOperands(), callOp->getAttrs());
+ callOp, TypeRange(), adaptor.getOperands(), callOp->getAttrs());
newOp.getProperties().operandSegmentSizes = {
static_cast<int32_t>(adaptor.getOperands().size()), 0};
newOp.getProperties().op_bundle_sizes = rewriter.getDenseI32ArrayAttr({});
diff --git a/mlir/test/lib/Dialect/Test/TestPatterns.cpp b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
index d073843484d81..a4cb705e6c8ea 100644
--- a/mlir/test/lib/Dialect/Test/TestPatterns.cpp
+++ b/mlir/test/lib/Dialect/Test/TestPatterns.cpp
@@ -1008,7 +1008,7 @@ struct TestPassthroughInvalidOp : public ConversionPattern {
op->getOperand(it.index()).getType(), range)
.getResult());
}
- rewriter.replaceOpWithNewOp<TestValidOp>(op, std::nullopt, flattened,
+ rewriter.replaceOpWithNewOp<TestValidOp>(op, TypeRange(), flattened,
std::nullopt);
return success();
}
@@ -1024,7 +1024,7 @@ struct TestDropAndReplaceInvalidOp : public ConversionPattern {
LogicalResult
matchAndRewrite(Operation *op, ArrayRef<Value> operands,
ConversionPatternRewriter &rewriter) const final {
- rewriter.replaceOpWithNewOp<TestValidOp>(op, std::nullopt, ValueRange(),
+ rewriter.replaceOpWithNewOp<TestValidOp>(op, TypeRange(), ValueRange(),
std::nullopt);
return success();
}
|
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 uses of std::nullopt is in a the constructors for TypeRange. This patch takes care of the migration where we need TypeRange() to facilitate perfect forwarding. Note that {} would be ambiguous for perfecting forwarding to work.
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 uses of std::nullopt is in a the constructors for TypeRange. This patch takes care of the migration where we need TypeRange() to facilitate perfect forwarding. Note that {} would be ambiguous for perfecting forwarding to work.
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 uses of std::nullopt is in a the constructors for
TypeRange. This patch takes care of the migration where we need
TypeRange() to facilitate perfect forwarding. Note that {} would be
ambiguous for perfecting forwarding to work.