Skip to content

[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

Conversation

kazutakahirata
Copy link
Contributor

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.
@llvmbot
Copy link
Member

llvmbot commented Jun 22, 2025

@llvm/pr-subscribers-mlir-spirv
@llvm/pr-subscribers-mlir-gpu

@llvm/pr-subscribers-mlir

Author: Kazu Hirata (kazutakahirata)

Changes

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.


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

3 Files Affected:

  • (modified) mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp (+1-1)
  • (modified) mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp (+1-1)
  • (modified) mlir/test/lib/Dialect/Test/TestPatterns.cpp (+2-2)
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();
   }

@kazutakahirata kazutakahirata merged commit 6023ba2 into llvm:main Jun 23, 2025
11 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_20250622_nullopt_mlir_TypeRange branch June 23, 2025 02:10
miguelcsx pushed a commit to miguelcsx/llvm-project that referenced this pull request Jun 23, 2025
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.
Jaddyen pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 23, 2025
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants