[Mlir-commits] [mlir] [mlir][acc] Introduce createAndPopulate for recipe creation (PR #162917)
Razvan Lupusoru
llvmlistbot at llvm.org
Tue Oct 14 09:56:21 PDT 2025
https://github.com/razvanlupusoru updated https://github.com/llvm/llvm-project/pull/162917
>From 156cc698541aceb42fd78695541fe52c95b50b75 Mon Sep 17 00:00:00 2001
From: Razvan Lupusoru <rlupusoru at nvidia.com>
Date: Fri, 10 Oct 2025 13:25:51 -0700
Subject: [PATCH 1/6] [mlir][acc] Introduce createAndPopulate for recipe
creation
Private and firstprivate recipes can now be created and populated
through the createAndPopulate method. The populating of recipe
bodies is done through using the PointerLikeType and MappableType
interfaces (with MappableType support still in progress).
The existing create() API remains available for cases where dialects
need manual recipe population (e.g., for frontend-specific semantics
like default value initialization or constructor calls).
Testing exercises the new API with memref types.
---
.../mlir/Dialect/OpenACC/OpenACCOps.td | 32 ++
.../Dialect/OpenACC/OpenACCTypeInterfaces.td | 44 ++-
mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp | 282 +++++++++++++++++-
.../OpenACC/recipe-populate-firstprivate.mlir | 102 +++++++
.../OpenACC/recipe-populate-private.mlir | 82 +++++
mlir/test/lib/Dialect/OpenACC/CMakeLists.txt | 1 +
mlir/test/lib/Dialect/OpenACC/TestOpenACC.cpp | 6 +-
.../OpenACC/TestPointerLikeTypeInterface.cpp | 7 +-
.../Dialect/OpenACC/TestRecipePopulate.cpp | 110 +++++++
9 files changed, 644 insertions(+), 22 deletions(-)
create mode 100644 mlir/test/Dialect/OpenACC/recipe-populate-firstprivate.mlir
create mode 100644 mlir/test/Dialect/OpenACC/recipe-populate-private.mlir
create mode 100644 mlir/test/lib/Dialect/OpenACC/TestRecipePopulate.cpp
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index 77e833f8f9492..c22c0dec6a2a1 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -1316,6 +1316,22 @@ def OpenACC_PrivateRecipeOp
}];
let hasRegionVerifier = 1;
+
+ let extraClassDeclaration = [{
+ /// Creates a PrivateRecipeOp and populates its regions based on the
+ /// variable type as long as the type implements MappableType or
+ /// PointerLikeType interface. If a type implements both, the MappableType
+ /// API will be preferred. Returns std::nullopt if the recipe cannot be
+ /// created or populated. The builder's current insertion point will be used
+ /// and it must be a valid place for this operation to be inserted.
+ static std::optional<PrivateRecipeOp> createAndPopulate(
+ ::mlir::OpBuilder &builder,
+ ::mlir::Location loc,
+ ::llvm::StringRef recipeName,
+ ::mlir::Value var,
+ ::llvm::StringRef varName = "",
+ ::mlir::ValueRange bounds = {});
+ }];
}
//===----------------------------------------------------------------------===//
@@ -1410,6 +1426,22 @@ def OpenACC_FirstprivateRecipeOp
}];
let hasRegionVerifier = 1;
+
+ let extraClassDeclaration = [{
+ /// Creates a FirstprivateRecipeOp and populates its regions based on the
+ /// variable type as long as the type implements MappableType or
+ /// PointerLikeType interface. If a type implements both, the MappableType
+ /// API will be preferred. Returns std::nullopt if the recipe cannot be
+ /// created or populated. The builder's current insertion point will be used
+ /// and it must be a valid place for this operation to be inserted.
+ static std::optional<FirstprivateRecipeOp> createAndPopulate(
+ ::mlir::OpBuilder &builder,
+ ::mlir::Location loc,
+ ::llvm::StringRef recipeName,
+ ::mlir::Value var,
+ ::llvm::StringRef varName = "",
+ ::mlir::ValueRange bounds = {});
+ }];
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCTypeInterfaces.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCTypeInterfaces.td
index 0d16255c5a994..684cf70b5c045 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCTypeInterfaces.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCTypeInterfaces.td
@@ -83,7 +83,15 @@ def OpenACC_PointerLikeTypeInterface : TypeInterface<"PointerLikeType"> {
The `originalVar` parameter is optional but enables support for dynamic
types (e.g., dynamic memrefs). When provided, implementations can extract
runtime dimension information from the original variable to create
- allocations with matching dynamic sizes.
+ allocations with matching dynamic sizes. When generating recipe bodies,
+ `originalVar` should be the block argument representing the original
+ variable in the recipe region.
+
+ The `needsFree` output parameter indicates whether the allocated memory
+ requires explicit deallocation. Implementations should set this to true
+ for heap allocations that need a matching deallocation operation (e.g.,
+ alloc) and false for stack-based allocations (e.g., alloca). During
+ recipe generation, this determines whether a destroy region is created.
Returns a Value representing the result of the allocation. If no value
is returned, it means the allocation was not successfully generated.
@@ -94,7 +102,8 @@ def OpenACC_PointerLikeTypeInterface : TypeInterface<"PointerLikeType"> {
"::mlir::Location":$loc,
"::llvm::StringRef":$varName,
"::mlir::Type":$varType,
- "::mlir::Value":$originalVar),
+ "::mlir::Value":$originalVar,
+ "bool &":$needsFree),
/*methodBody=*/"",
/*defaultImplementation=*/[{
return {};
@@ -102,23 +111,34 @@ def OpenACC_PointerLikeTypeInterface : TypeInterface<"PointerLikeType"> {
>,
InterfaceMethod<
/*description=*/[{
- Generates deallocation operations for the pointer-like type. It deallocates
- the instance provided.
+ Generates deallocation operations for the pointer-like type.
- The `varPtr` parameter is required and must represent an instance that was
- previously allocated. If the current type is represented in a way that it
- does not capture the pointee type, `varType` must be passed in to provide
- the necessary type information. Nothing is generated in case the allocate
- is `alloca`-like.
+ The `varToFree` parameter is required and must represent an instance
+ that was previously allocated. When generating recipe bodies, this
+ should be the block argument representing the private variable in the
+ destroy region.
+
+ The `allocRes` parameter is optional and provides the result of the
+ corresponding allocation from the init region. This allows implementations
+ to inspect the allocation operation to determine the appropriate
+ deallocation strategy. This is necessary because in recipe generation,
+ the allocation and deallocation occur in separate regions. Dialects that
+ use only one allocation type or can determine deallocation from type
+ information alone may ignore this parameter.
+
+ The `varType` parameter must be provided if the current type does not
+ capture the pointee type information. No deallocation is generated for
+ stack-based allocations (e.g., alloca).
- Returns true if deallocation was successfully generated or successfully
- deemed as not needed to be generated, false otherwise.
+ Returns true if deallocation was successfully generated or determined to
+ be unnecessary, false otherwise.
}],
/*retTy=*/"bool",
/*methodName=*/"genFree",
/*args=*/(ins "::mlir::OpBuilder &":$builder,
"::mlir::Location":$loc,
- "::mlir::TypedValue<::mlir::acc::PointerLikeType>":$varPtr,
+ "::mlir::TypedValue<::mlir::acc::PointerLikeType>":$varToFree,
+ "::mlir::Value":$allocRes,
"::mlir::Type":$varType),
/*methodBody=*/"",
/*defaultImplementation=*/[{
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index 6564a4ecdccd3..be9f9342f7f18 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -74,14 +74,16 @@ struct MemRefPointerLikeModel
}
mlir::Value genAllocate(Type pointer, OpBuilder &builder, Location loc,
- StringRef varName, Type varType,
- Value originalVar) const {
+ StringRef varName, Type varType, Value originalVar,
+ bool &needsFree) const {
auto memrefTy = cast<MemRefType>(pointer);
// Check if this is a static memref (all dimensions are known) - if yes
// then we can generate an alloca operation.
- if (memrefTy.hasStaticShape())
+ if (memrefTy.hasStaticShape()) {
+ needsFree = false; // alloca doesn't need deallocation
return memref::AllocaOp::create(builder, loc, memrefTy).getResult();
+ }
// For dynamic memrefs, extract sizes from the original variable if
// provided. Otherwise they cannot be handled.
@@ -99,6 +101,7 @@ struct MemRefPointerLikeModel
// Note: We only add dynamic sizes to the dynamicSizes array
// Static dimensions are handled automatically by AllocOp
}
+ needsFree = true; // alloc needs deallocation
return memref::AllocOp::create(builder, loc, memrefTy, dynamicSizes)
.getResult();
}
@@ -108,10 +111,14 @@ struct MemRefPointerLikeModel
}
bool genFree(Type pointer, OpBuilder &builder, Location loc,
- TypedValue<PointerLikeType> varPtr, Type varType) const {
- if (auto memrefValue = dyn_cast<TypedValue<MemRefType>>(varPtr)) {
+ TypedValue<PointerLikeType> varToFree, Value allocRes,
+ Type varType) const {
+ if (auto memrefValue = dyn_cast<TypedValue<MemRefType>>(varToFree)) {
+ // Use allocRes if provided to determine the allocation type
+ Value valueToInspect = allocRes ? allocRes : memrefValue;
+
// Walk through casts to find the original allocation
- Value currentValue = memrefValue;
+ Value currentValue = valueToInspect;
Operation *originalAlloc = nullptr;
// Follow the chain of operations to find the original allocation
@@ -150,7 +157,7 @@ struct MemRefPointerLikeModel
return true;
}
if (isa<memref::AllocOp>(originalAlloc)) {
- // This is an alloc - generate dealloc
+ // This is an alloc - generate dealloc on varToFree
memref::DeallocOp::create(builder, loc, memrefValue);
return true;
}
@@ -1003,6 +1010,151 @@ struct RemoveConstantIfConditionWithRegion : public OpRewritePattern<OpTy> {
}
};
+//===----------------------------------------------------------------------===//
+// Recipe Region Helpers
+//===----------------------------------------------------------------------===//
+
+/// Create and populate an init region for privatization recipes.
+/// Returns the init block on success, or nullptr on failure.
+/// Sets needsFree to indicate if the allocated memory requires deallocation.
+static std::unique_ptr<Block> createInitRegion(OpBuilder &builder, Location loc,
+ Value var, StringRef varName,
+ ValueRange bounds, Type varType,
+ bool &needsFree) {
+ // Create init block with arguments: original value + bounds
+ SmallVector<Type> argTypes{varType};
+ SmallVector<Location> argLocs{loc};
+ for (Value bound : bounds) {
+ argTypes.push_back(bound.getType());
+ argLocs.push_back(loc);
+ }
+
+ auto initBlock = std::make_unique<Block>();
+ initBlock->addArguments(argTypes, argLocs);
+ builder.setInsertionPointToStart(initBlock.get());
+
+ Value privatizedValue;
+
+ // Get the block argument that represents the original variable
+ Value blockArgVar = initBlock->getArgument(0);
+
+ // Generate init region body based on variable type
+ if (isa<MappableType>(varType)) {
+ auto mappableTy = cast<MappableType>(varType);
+ auto typedVar = cast<TypedValue<MappableType>>(blockArgVar);
+ privatizedValue = mappableTy.generatePrivateInit(builder, loc, typedVar,
+ varName, bounds, {});
+ if (!privatizedValue) {
+ return nullptr;
+ }
+ // TODO: MappableType doesn't yet support needsFree
+ // For now assume it doesn't need explicit deallocation
+ needsFree = false;
+ } else {
+ assert(isa<PointerLikeType>(varType) && "Expected PointerLikeType");
+ auto pointerLikeTy = cast<PointerLikeType>(varType);
+ // Use PointerLikeType's allocation API with the block argument
+ privatizedValue = pointerLikeTy.genAllocate(builder, loc, varName, varType,
+ blockArgVar, needsFree);
+ if (!privatizedValue) {
+ return nullptr;
+ }
+ }
+
+ // Add yield operation to init block
+ acc::YieldOp::create(builder, loc, privatizedValue);
+
+ return initBlock;
+}
+
+/// Create and populate a copy region for firstprivate recipes.
+/// Returns the copy block on success, or nullptr on failure.
+/// TODO: Handle MappableType - it does not yet have a copy API.
+static std::unique_ptr<Block> createCopyRegion(OpBuilder &builder, Location loc,
+ ValueRange bounds,
+ Type varType) {
+ // Create copy block with arguments: original value + privatized value +
+ // bounds
+ SmallVector<Type> copyArgTypes{varType, varType};
+ SmallVector<Location> copyArgLocs{loc, loc};
+ for (Value bound : bounds) {
+ copyArgTypes.push_back(bound.getType());
+ copyArgLocs.push_back(loc);
+ }
+
+ auto copyBlock = std::make_unique<Block>();
+ copyBlock->addArguments(copyArgTypes, copyArgLocs);
+ builder.setInsertionPointToStart(copyBlock.get());
+
+ bool isMappable = isa<MappableType>(varType);
+ bool isPointerLike = isa<PointerLikeType>(varType);
+ if (isMappable && !isPointerLike) {
+ // TODO: Handle MappableType - it does not yet have a copy API.
+ // Otherwise, for now just fallback to pointer-like behavior.
+ return nullptr;
+ }
+
+ // Generate copy region body based on variable type
+ if (isPointerLike) {
+ auto pointerLikeTy = cast<PointerLikeType>(varType);
+ Value originalArg = copyBlock->getArgument(0);
+ Value privatizedArg = copyBlock->getArgument(1);
+
+ // Generate copy operation using PointerLikeType interface
+ if (!pointerLikeTy.genCopy(
+ builder, loc, cast<TypedValue<PointerLikeType>>(privatizedArg),
+ cast<TypedValue<PointerLikeType>>(originalArg), varType)) {
+ return nullptr;
+ }
+ }
+
+ // Add terminator to copy block
+ acc::TerminatorOp::create(builder, loc);
+
+ return copyBlock;
+}
+
+/// Create and populate a destroy region for privatization recipes.
+/// Returns the destroy block on success, or nullptr if not needed.
+static std::unique_ptr<Block> createDestroyRegion(OpBuilder &builder,
+ Location loc, Value allocRes,
+ ValueRange bounds,
+ Type varType) {
+ // Create destroy block with arguments: original value + privatized value +
+ // bounds
+ SmallVector<Type> destroyArgTypes{varType, varType};
+ SmallVector<Location> destroyArgLocs{loc, loc};
+ for (Value bound : bounds) {
+ destroyArgTypes.push_back(bound.getType());
+ destroyArgLocs.push_back(loc);
+ }
+
+ auto destroyBlock = std::make_unique<Block>();
+ destroyBlock->addArguments(destroyArgTypes, destroyArgLocs);
+ builder.setInsertionPointToStart(destroyBlock.get());
+
+ bool isMappable = isa<MappableType>(varType);
+ bool isPointerLike = isa<PointerLikeType>(varType);
+ if (isMappable && !isPointerLike) {
+ // TODO: Handle MappableType - it does not yet have a deallocation API.
+ // Otherwise, for now just fallback to pointer-like behavior.
+ return nullptr;
+ }
+
+ assert(isa<PointerLikeType>(varType) && "Expected PointerLikeType");
+ auto pointerLikeTy = cast<PointerLikeType>(varType);
+ auto privatizedArg =
+ cast<TypedValue<PointerLikeType>>(destroyBlock->getArgument(1));
+ // Pass allocRes to help determine the allocation type
+ if (!pointerLikeTy.genFree(builder, loc, privatizedArg, allocRes, varType)) {
+ return nullptr;
+ }
+
+ acc::TerminatorOp::create(builder, loc);
+
+ return destroyBlock;
+}
+
} // namespace
//===----------------------------------------------------------------------===//
@@ -1050,6 +1202,61 @@ LogicalResult acc::PrivateRecipeOp::verifyRegions() {
return success();
}
+std::optional<PrivateRecipeOp>
+PrivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
+ StringRef recipeName, Value var,
+ StringRef varName, ValueRange bounds) {
+
+ Type varType = var.getType();
+
+ // First, validate that we can handle this variable type
+ bool isMappable = isa<MappableType>(varType);
+ bool isPointerLike = isa<PointerLikeType>(varType);
+
+ if (!isMappable && !isPointerLike) {
+ // Unsupported type
+ return std::nullopt;
+ }
+
+ // Create init and destroy blocks using shared helpers
+ OpBuilder::InsertionGuard guard(builder);
+
+ // Save the original insertion point for creating the recipe operation later
+ auto originalInsertionPoint = builder.saveInsertionPoint();
+
+ bool needsFree = false;
+ auto initBlock =
+ createInitRegion(builder, loc, var, varName, bounds, varType, needsFree);
+ if (!initBlock) {
+ return std::nullopt;
+ }
+
+ // Only create destroy region if the allocation needs deallocation
+ std::unique_ptr<Block> destroyBlock;
+ if (needsFree) {
+ // Extract the allocated value from the init block's yield operation
+ auto yieldOp = cast<acc::YieldOp>(initBlock->getTerminator());
+ Value allocRes = yieldOp.getOperand(0);
+
+ destroyBlock = createDestroyRegion(builder, loc, allocRes, bounds, varType);
+ if (!destroyBlock) {
+ return std::nullopt;
+ }
+ }
+
+ // Now create the recipe operation at the original insertion point and attach
+ // the blocks
+ builder.restoreInsertionPoint(originalInsertionPoint);
+ auto recipe = PrivateRecipeOp::create(builder, loc, recipeName, varType);
+
+ // Move the blocks into the recipe's regions
+ recipe.getInitRegion().push_back(initBlock.release());
+ if (destroyBlock)
+ recipe.getDestroyRegion().push_back(destroyBlock.release());
+
+ return recipe;
+}
+
//===----------------------------------------------------------------------===//
// FirstprivateRecipeOp
//===----------------------------------------------------------------------===//
@@ -1080,6 +1287,67 @@ LogicalResult acc::FirstprivateRecipeOp::verifyRegions() {
return success();
}
+std::optional<FirstprivateRecipeOp>
+FirstprivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
+ StringRef recipeName, Value var,
+ StringRef varName, ValueRange bounds) {
+
+ Type varType = var.getType();
+
+ // First, validate that we can handle this variable type
+ bool isMappable = isa<MappableType>(varType);
+ bool isPointerLike = isa<PointerLikeType>(varType);
+
+ if (!isMappable && !isPointerLike) {
+ // Unsupported type
+ return std::nullopt;
+ }
+
+ // Create init, copy, and destroy blocks using shared helpers
+ OpBuilder::InsertionGuard guard(builder);
+
+ // Save the original insertion point for creating the recipe operation later
+ auto originalInsertionPoint = builder.saveInsertionPoint();
+
+ bool needsFree = false;
+ auto initBlock =
+ createInitRegion(builder, loc, var, varName, bounds, varType, needsFree);
+ if (!initBlock) {
+ return std::nullopt;
+ }
+
+ auto copyBlock = createCopyRegion(builder, loc, bounds, varType);
+ if (!copyBlock) {
+ return std::nullopt;
+ }
+
+ // Only create destroy region if the allocation needs deallocation
+ std::unique_ptr<Block> destroyBlock;
+ if (needsFree) {
+ // Extract the allocated value from the init block's yield operation
+ auto yieldOp = cast<acc::YieldOp>(initBlock->getTerminator());
+ Value allocRes = yieldOp.getOperand(0);
+
+ destroyBlock = createDestroyRegion(builder, loc, allocRes, bounds, varType);
+ if (!destroyBlock) {
+ return std::nullopt;
+ }
+ }
+
+ // Now create the recipe operation at the original insertion point and attach
+ // the blocks
+ builder.restoreInsertionPoint(originalInsertionPoint);
+ auto recipe = FirstprivateRecipeOp::create(builder, loc, recipeName, varType);
+
+ // Move the blocks into the recipe's regions
+ recipe.getInitRegion().push_back(initBlock.release());
+ recipe.getCopyRegion().push_back(copyBlock.release());
+ if (destroyBlock)
+ recipe.getDestroyRegion().push_back(destroyBlock.release());
+
+ return recipe;
+}
+
//===----------------------------------------------------------------------===//
// ReductionRecipeOp
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/OpenACC/recipe-populate-firstprivate.mlir b/mlir/test/Dialect/OpenACC/recipe-populate-firstprivate.mlir
new file mode 100644
index 0000000000000..35355c6e06164
--- /dev/null
+++ b/mlir/test/Dialect/OpenACC/recipe-populate-firstprivate.mlir
@@ -0,0 +1,102 @@
+// RUN: mlir-opt %s --split-input-file --pass-pipeline="builtin.module(test-acc-recipe-populate{recipe-type=firstprivate})" | FileCheck %s
+
+// CHECK: acc.firstprivate.recipe @firstprivate_scalar : memref<f32> init {
+// CHECK: ^bb0(%{{.*}}: memref<f32>):
+// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref<f32>
+// CHECK: acc.yield %[[ALLOC]] : memref<f32>
+// CHECK: } copy {
+// CHECK: ^bb0(%[[SRC:.*]]: memref<f32>, %[[DST:.*]]: memref<f32>):
+// CHECK: memref.copy %[[SRC]], %[[DST]] : memref<f32> to memref<f32>
+// CHECK: acc.terminator
+// CHECK: }
+// CHECK-NOT: destroy
+
+func.func @test_scalar() {
+ %0 = memref.alloca() {test.var = "scalar"} : memref<f32>
+ return
+}
+
+// -----
+
+// CHECK: acc.firstprivate.recipe @firstprivate_static_2d : memref<10x20xf32> init {
+// CHECK: ^bb0(%{{.*}}: memref<10x20xf32>):
+// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref<10x20xf32>
+// CHECK: acc.yield %[[ALLOC]] : memref<10x20xf32>
+// CHECK: } copy {
+// CHECK: ^bb0(%[[SRC:.*]]: memref<10x20xf32>, %[[DST:.*]]: memref<10x20xf32>):
+// CHECK: memref.copy %[[SRC]], %[[DST]] : memref<10x20xf32> to memref<10x20xf32>
+// CHECK: acc.terminator
+// CHECK: }
+// CHECK-NOT: destroy
+
+func.func @test_static_2d() {
+ %0 = memref.alloca() {test.var = "static_2d"} : memref<10x20xf32>
+ return
+}
+
+// -----
+
+// CHECK: acc.firstprivate.recipe @firstprivate_dynamic_2d : memref<?x?xf32> init {
+// CHECK: ^bb0(%[[ARG:.*]]: memref<?x?xf32>):
+// CHECK: %[[C0:.*]] = arith.constant 0 : index
+// CHECK: %[[DIM0:.*]] = memref.dim %[[ARG]], %[[C0]] : memref<?x?xf32>
+// CHECK: %[[C1:.*]] = arith.constant 1 : index
+// CHECK: %[[DIM1:.*]] = memref.dim %[[ARG]], %[[C1]] : memref<?x?xf32>
+// CHECK: %[[ALLOC:.*]] = memref.alloc(%[[DIM0]], %[[DIM1]]) : memref<?x?xf32>
+// CHECK: acc.yield %[[ALLOC]] : memref<?x?xf32>
+// CHECK: } copy {
+// CHECK: ^bb0(%[[SRC:.*]]: memref<?x?xf32>, %[[DST:.*]]: memref<?x?xf32>):
+// CHECK: memref.copy %[[SRC]], %[[DST]] : memref<?x?xf32> to memref<?x?xf32>
+// CHECK: acc.terminator
+// CHECK: } destroy {
+// CHECK: ^bb0(%{{.*}}: memref<?x?xf32>, %[[VAL:.*]]: memref<?x?xf32>):
+// CHECK: memref.dealloc %[[VAL]] : memref<?x?xf32>
+// CHECK: acc.terminator
+// CHECK: }
+
+func.func @test_dynamic_2d(%arg0: index, %arg1: index) {
+ %0 = memref.alloc(%arg0, %arg1) {test.var = "dynamic_2d"} : memref<?x?xf32>
+ return
+}
+
+// -----
+
+// CHECK: acc.firstprivate.recipe @firstprivate_mixed_dims : memref<10x?xf32> init {
+// CHECK: ^bb0(%[[ARG:.*]]: memref<10x?xf32>):
+// CHECK: %[[C1:.*]] = arith.constant 1 : index
+// CHECK: %[[DIM1:.*]] = memref.dim %[[ARG]], %[[C1]] : memref<10x?xf32>
+// CHECK: %[[ALLOC:.*]] = memref.alloc(%[[DIM1]]) : memref<10x?xf32>
+// CHECK: acc.yield %[[ALLOC]] : memref<10x?xf32>
+// CHECK: } copy {
+// CHECK: ^bb0(%[[SRC:.*]]: memref<10x?xf32>, %[[DST:.*]]: memref<10x?xf32>):
+// CHECK: memref.copy %[[SRC]], %[[DST]] : memref<10x?xf32> to memref<10x?xf32>
+// CHECK: acc.terminator
+// CHECK: } destroy {
+// CHECK: ^bb0(%{{.*}}: memref<10x?xf32>, %[[VAL:.*]]: memref<10x?xf32>):
+// CHECK: memref.dealloc %[[VAL]] : memref<10x?xf32>
+// CHECK: acc.terminator
+// CHECK: }
+
+func.func @test_mixed_dims(%arg0: index) {
+ %0 = memref.alloc(%arg0) {test.var = "mixed_dims"} : memref<10x?xf32>
+ return
+}
+
+// -----
+
+// CHECK: acc.firstprivate.recipe @firstprivate_scalar_int : memref<i32> init {
+// CHECK: ^bb0(%{{.*}}: memref<i32>):
+// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref<i32>
+// CHECK: acc.yield %[[ALLOC]] : memref<i32>
+// CHECK: } copy {
+// CHECK: ^bb0(%[[SRC:.*]]: memref<i32>, %[[DST:.*]]: memref<i32>):
+// CHECK: memref.copy %[[SRC]], %[[DST]] : memref<i32> to memref<i32>
+// CHECK: acc.terminator
+// CHECK: }
+// CHECK-NOT: destroy
+
+func.func @test_scalar_int() {
+ %0 = memref.alloca() {test.var = "scalar_int"} : memref<i32>
+ return
+}
+
diff --git a/mlir/test/Dialect/OpenACC/recipe-populate-private.mlir b/mlir/test/Dialect/OpenACC/recipe-populate-private.mlir
new file mode 100644
index 0000000000000..8403ee80a7bc6
--- /dev/null
+++ b/mlir/test/Dialect/OpenACC/recipe-populate-private.mlir
@@ -0,0 +1,82 @@
+// RUN: mlir-opt %s --split-input-file --pass-pipeline="builtin.module(test-acc-recipe-populate{recipe-type=private})" | FileCheck %s
+
+// CHECK: acc.private.recipe @private_scalar : memref<f32> init {
+// CHECK: ^bb0(%{{.*}}: memref<f32>):
+// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref<f32>
+// CHECK: acc.yield %[[ALLOC]] : memref<f32>
+// CHECK: }
+// CHECK-NOT: destroy
+
+func.func @test_scalar() {
+ %0 = memref.alloca() {test.var = "scalar"} : memref<f32>
+ return
+}
+
+// -----
+
+// CHECK: acc.private.recipe @private_static_2d : memref<10x20xf32> init {
+// CHECK: ^bb0(%{{.*}}: memref<10x20xf32>):
+// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref<10x20xf32>
+// CHECK: acc.yield %[[ALLOC]] : memref<10x20xf32>
+// CHECK: }
+// CHECK-NOT: destroy
+
+func.func @test_static_2d() {
+ %0 = memref.alloca() {test.var = "static_2d"} : memref<10x20xf32>
+ return
+}
+
+// -----
+
+// CHECK: acc.private.recipe @private_dynamic_2d : memref<?x?xf32> init {
+// CHECK: ^bb0(%[[ARG:.*]]: memref<?x?xf32>):
+// CHECK: %[[C0:.*]] = arith.constant 0 : index
+// CHECK: %[[DIM0:.*]] = memref.dim %[[ARG]], %[[C0]] : memref<?x?xf32>
+// CHECK: %[[C1:.*]] = arith.constant 1 : index
+// CHECK: %[[DIM1:.*]] = memref.dim %[[ARG]], %[[C1]] : memref<?x?xf32>
+// CHECK: %[[ALLOC:.*]] = memref.alloc(%[[DIM0]], %[[DIM1]]) : memref<?x?xf32>
+// CHECK: acc.yield %[[ALLOC]] : memref<?x?xf32>
+// CHECK: } destroy {
+// CHECK: ^bb0(%{{.*}}: memref<?x?xf32>, %[[VAL:.*]]: memref<?x?xf32>):
+// CHECK: memref.dealloc %[[VAL]] : memref<?x?xf32>
+// CHECK: acc.terminator
+// CHECK: }
+
+func.func @test_dynamic_2d(%arg0: index, %arg1: index) {
+ %0 = memref.alloc(%arg0, %arg1) {test.var = "dynamic_2d"} : memref<?x?xf32>
+ return
+}
+
+// -----
+
+// CHECK: acc.private.recipe @private_mixed_dims : memref<10x?xf32> init {
+// CHECK: ^bb0(%[[ARG:.*]]: memref<10x?xf32>):
+// CHECK: %[[C1:.*]] = arith.constant 1 : index
+// CHECK: %[[DIM1:.*]] = memref.dim %[[ARG]], %[[C1]] : memref<10x?xf32>
+// CHECK: %[[ALLOC:.*]] = memref.alloc(%[[DIM1]]) : memref<10x?xf32>
+// CHECK: acc.yield %[[ALLOC]] : memref<10x?xf32>
+// CHECK: } destroy {
+// CHECK: ^bb0(%{{.*}}: memref<10x?xf32>, %[[VAL:.*]]: memref<10x?xf32>):
+// CHECK: memref.dealloc %[[VAL]] : memref<10x?xf32>
+// CHECK: acc.terminator
+// CHECK: }
+
+func.func @test_mixed_dims(%arg0: index) {
+ %0 = memref.alloc(%arg0) {test.var = "mixed_dims"} : memref<10x?xf32>
+ return
+}
+
+// -----
+
+// CHECK: acc.private.recipe @private_scalar_int : memref<i32> init {
+// CHECK: ^bb0(%{{.*}}: memref<i32>):
+// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref<i32>
+// CHECK: acc.yield %[[ALLOC]] : memref<i32>
+// CHECK: }
+// CHECK-NOT: destroy
+
+func.func @test_scalar_int() {
+ %0 = memref.alloca() {test.var = "scalar_int"} : memref<i32>
+ return
+}
+
diff --git a/mlir/test/lib/Dialect/OpenACC/CMakeLists.txt b/mlir/test/lib/Dialect/OpenACC/CMakeLists.txt
index f84055df1b6c6..1e593389ec683 100644
--- a/mlir/test/lib/Dialect/OpenACC/CMakeLists.txt
+++ b/mlir/test/lib/Dialect/OpenACC/CMakeLists.txt
@@ -1,6 +1,7 @@
add_mlir_library(MLIROpenACCTestPasses
TestOpenACC.cpp
TestPointerLikeTypeInterface.cpp
+ TestRecipePopulate.cpp
EXCLUDE_FROM_LIBMLIR
)
diff --git a/mlir/test/lib/Dialect/OpenACC/TestOpenACC.cpp b/mlir/test/lib/Dialect/OpenACC/TestOpenACC.cpp
index 98862401748e1..bea21b9827f7e 100644
--- a/mlir/test/lib/Dialect/OpenACC/TestOpenACC.cpp
+++ b/mlir/test/lib/Dialect/OpenACC/TestOpenACC.cpp
@@ -15,9 +15,13 @@ namespace test {
// Forward declarations of individual test pass registration functions
void registerTestPointerLikeTypeInterfacePass();
+void registerTestRecipePopulatePass();
// Unified registration function for all OpenACC tests
-void registerTestOpenACC() { registerTestPointerLikeTypeInterfacePass(); }
+void registerTestOpenACC() {
+ registerTestPointerLikeTypeInterfacePass();
+ registerTestRecipePopulatePass();
+}
} // namespace test
} // namespace mlir
diff --git a/mlir/test/lib/Dialect/OpenACC/TestPointerLikeTypeInterface.cpp b/mlir/test/lib/Dialect/OpenACC/TestPointerLikeTypeInterface.cpp
index 85f92833a3269..88cb5b72aacd2 100644
--- a/mlir/test/lib/Dialect/OpenACC/TestPointerLikeTypeInterface.cpp
+++ b/mlir/test/lib/Dialect/OpenACC/TestPointerLikeTypeInterface.cpp
@@ -196,13 +196,15 @@ void TestPointerLikeTypeInterfacePass::testGenAllocate(
newBuilder.setInsertionPointAfter(op);
// Call the genAllocate API
+ bool needsFree = false;
Value allocRes = pointerType.genAllocate(newBuilder, loc, "test_alloc",
- result.getType(), result);
+ result.getType(), result, needsFree);
if (allocRes) {
llvm::errs() << "Successfully generated alloc for operation: ";
op->print(llvm::errs());
llvm::errs() << "\n";
+ llvm::errs() << "\tneeds free: " << (needsFree ? "true" : "false") << "\n";
// Print all operations that were inserted
for (Operation *insertedOp : tracker.insertedOps) {
@@ -230,8 +232,9 @@ void TestPointerLikeTypeInterfacePass::testGenFree(Operation *op, Value result,
// Call the genFree API
auto typedResult = cast<TypedValue<PointerLikeType>>(result);
+ // In this test context, we don't have the allocation result, so pass the result itself
bool success =
- pointerType.genFree(newBuilder, loc, typedResult, result.getType());
+ pointerType.genFree(newBuilder, loc, typedResult, result, result.getType());
if (success) {
llvm::errs() << "Successfully generated free for operation: ";
diff --git a/mlir/test/lib/Dialect/OpenACC/TestRecipePopulate.cpp b/mlir/test/lib/Dialect/OpenACC/TestRecipePopulate.cpp
new file mode 100644
index 0000000000000..d3664b6eebbd4
--- /dev/null
+++ b/mlir/test/lib/Dialect/OpenACC/TestRecipePopulate.cpp
@@ -0,0 +1,110 @@
+//===- TestRecipePopulate.cpp - Test Recipe Population -------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file contains test passes for testing the createAndPopulate methods
+// of the recipe operations.
+//
+//===----------------------------------------------------------------------===//
+
+#include "mlir/Dialect/Arith/IR/Arith.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/MemRef/IR/MemRef.h"
+#include "mlir/Dialect/OpenACC/OpenACC.h"
+#include "mlir/IR/Builders.h"
+#include "mlir/Pass/Pass.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace mlir;
+using namespace mlir::acc;
+
+namespace {
+
+struct TestRecipePopulatePass
+ : public PassWrapper<TestRecipePopulatePass, OperationPass<ModuleOp>> {
+ MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestRecipePopulatePass)
+
+ TestRecipePopulatePass() = default;
+ TestRecipePopulatePass(const TestRecipePopulatePass &pass)
+ : PassWrapper(pass) {
+ recipeType = pass.recipeType;
+ }
+
+ Pass::Option<std::string> recipeType{
+ *this, "recipe-type",
+ llvm::cl::desc("Recipe type: private or firstprivate"),
+ llvm::cl::init("private")};
+
+ StringRef getArgument() const override { return "test-acc-recipe-populate"; }
+
+ StringRef getDescription() const override {
+ return "Test OpenACC recipe population";
+ }
+
+ void runOnOperation() override;
+
+ void getDependentDialects(DialectRegistry ®istry) const override {
+ registry.insert<acc::OpenACCDialect>();
+ registry.insert<arith::ArithDialect>();
+ registry.insert<memref::MemRefDialect>();
+ }
+};
+
+void TestRecipePopulatePass::runOnOperation() {
+ auto module = getOperation();
+ OpBuilder builder(&getContext());
+
+ // Collect all test variables
+ SmallVector<std::tuple<Operation *, Value, std::string>> testVars;
+
+ module.walk([&](Operation *op) {
+ if (auto varName = op->getAttrOfType<StringAttr>("test.var")) {
+ for (auto result : op->getResults()) {
+ testVars.push_back({op, result, varName.str()});
+ }
+ }
+ });
+
+ // Generate recipes at module level
+ builder.setInsertionPoint(&module.getBodyRegion().front(),
+ module.getBodyRegion().front().begin());
+
+ for (auto [op, var, varName] : testVars) {
+ Location loc = op->getLoc();
+
+ std::string recipeName = recipeType.getValue() + "_" + varName;
+ ValueRange bounds; // No bounds for memref tests
+
+ if (recipeType == "private") {
+ auto recipe = PrivateRecipeOp::createAndPopulate(builder, loc, recipeName,
+ var, varName, bounds);
+
+ if (!recipe) {
+ op->emitError("Failed to create private recipe for ") << varName;
+ }
+ } else if (recipeType == "firstprivate") {
+ auto recipe = FirstprivateRecipeOp::createAndPopulate(
+ builder, loc, recipeName, var, varName, bounds);
+
+ if (!recipe) {
+ op->emitError("Failed to create firstprivate recipe for ") << varName;
+ }
+ }
+ }
+}
+
+} // namespace
+
+namespace mlir {
+namespace test {
+
+void registerTestRecipePopulatePass() {
+ PassRegistration<TestRecipePopulatePass>();
+}
+
+} // namespace test
+} // namespace mlir
>From cf2dbb65d6525fc629476725ae69b249fde20064 Mon Sep 17 00:00:00 2001
From: Razvan Lupusoru <rlupusoru at nvidia.com>
Date: Fri, 10 Oct 2025 13:34:38 -0700
Subject: [PATCH 2/6] Fix formatting and remove useless comment
---
.../lib/Dialect/OpenACC/TestPointerLikeTypeInterface.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/mlir/test/lib/Dialect/OpenACC/TestPointerLikeTypeInterface.cpp b/mlir/test/lib/Dialect/OpenACC/TestPointerLikeTypeInterface.cpp
index 88cb5b72aacd2..027b0a1a8b80b 100644
--- a/mlir/test/lib/Dialect/OpenACC/TestPointerLikeTypeInterface.cpp
+++ b/mlir/test/lib/Dialect/OpenACC/TestPointerLikeTypeInterface.cpp
@@ -232,9 +232,8 @@ void TestPointerLikeTypeInterfacePass::testGenFree(Operation *op, Value result,
// Call the genFree API
auto typedResult = cast<TypedValue<PointerLikeType>>(result);
- // In this test context, we don't have the allocation result, so pass the result itself
- bool success =
- pointerType.genFree(newBuilder, loc, typedResult, result, result.getType());
+ bool success = pointerType.genFree(newBuilder, loc, typedResult, result,
+ result.getType());
if (success) {
llvm::errs() << "Successfully generated free for operation: ";
>From b6cc6e5793f17c02cd3b85c152b1ff3bf7d421a6 Mon Sep 17 00:00:00 2001
From: Razvan Lupusoru <rlupusoru at nvidia.com>
Date: Mon, 13 Oct 2025 12:55:30 -0700
Subject: [PATCH 3/6] Fix braces
---
mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp | 51 +++++++++----------------
1 file changed, 19 insertions(+), 32 deletions(-)
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index be9f9342f7f18..c568dece1fb0a 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -1044,9 +1044,8 @@ static std::unique_ptr<Block> createInitRegion(OpBuilder &builder, Location loc,
auto typedVar = cast<TypedValue<MappableType>>(blockArgVar);
privatizedValue = mappableTy.generatePrivateInit(builder, loc, typedVar,
varName, bounds, {});
- if (!privatizedValue) {
+ if (!privatizedValue)
return nullptr;
- }
// TODO: MappableType doesn't yet support needsFree
// For now assume it doesn't need explicit deallocation
needsFree = false;
@@ -1056,9 +1055,8 @@ static std::unique_ptr<Block> createInitRegion(OpBuilder &builder, Location loc,
// Use PointerLikeType's allocation API with the block argument
privatizedValue = pointerLikeTy.genAllocate(builder, loc, varName, varType,
blockArgVar, needsFree);
- if (!privatizedValue) {
+ if (!privatizedValue)
return nullptr;
- }
}
// Add yield operation to init block
@@ -1088,11 +1086,10 @@ static std::unique_ptr<Block> createCopyRegion(OpBuilder &builder, Location loc,
bool isMappable = isa<MappableType>(varType);
bool isPointerLike = isa<PointerLikeType>(varType);
- if (isMappable && !isPointerLike) {
- // TODO: Handle MappableType - it does not yet have a copy API.
- // Otherwise, for now just fallback to pointer-like behavior.
+ // TODO: Handle MappableType - it does not yet have a copy API.
+ // Otherwise, for now just fallback to pointer-like behavior.
+ if (isMappable && !isPointerLike)
return nullptr;
- }
// Generate copy region body based on variable type
if (isPointerLike) {
@@ -1103,9 +1100,8 @@ static std::unique_ptr<Block> createCopyRegion(OpBuilder &builder, Location loc,
// Generate copy operation using PointerLikeType interface
if (!pointerLikeTy.genCopy(
builder, loc, cast<TypedValue<PointerLikeType>>(privatizedArg),
- cast<TypedValue<PointerLikeType>>(originalArg), varType)) {
+ cast<TypedValue<PointerLikeType>>(originalArg), varType))
return nullptr;
- }
}
// Add terminator to copy block
@@ -1135,20 +1131,18 @@ static std::unique_ptr<Block> createDestroyRegion(OpBuilder &builder,
bool isMappable = isa<MappableType>(varType);
bool isPointerLike = isa<PointerLikeType>(varType);
- if (isMappable && !isPointerLike) {
- // TODO: Handle MappableType - it does not yet have a deallocation API.
- // Otherwise, for now just fallback to pointer-like behavior.
+ // TODO: Handle MappableType - it does not yet have a deallocation API.
+ // Otherwise, for now just fallback to pointer-like behavior.
+ if (isMappable && !isPointerLike)
return nullptr;
- }
assert(isa<PointerLikeType>(varType) && "Expected PointerLikeType");
auto pointerLikeTy = cast<PointerLikeType>(varType);
auto privatizedArg =
cast<TypedValue<PointerLikeType>>(destroyBlock->getArgument(1));
// Pass allocRes to help determine the allocation type
- if (!pointerLikeTy.genFree(builder, loc, privatizedArg, allocRes, varType)) {
+ if (!pointerLikeTy.genFree(builder, loc, privatizedArg, allocRes, varType))
return nullptr;
- }
acc::TerminatorOp::create(builder, loc);
@@ -1213,10 +1207,9 @@ PrivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
bool isMappable = isa<MappableType>(varType);
bool isPointerLike = isa<PointerLikeType>(varType);
- if (!isMappable && !isPointerLike) {
- // Unsupported type
+ // Unsupported type
+ if (!isMappable && !isPointerLike)
return std::nullopt;
- }
// Create init and destroy blocks using shared helpers
OpBuilder::InsertionGuard guard(builder);
@@ -1227,9 +1220,8 @@ PrivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
bool needsFree = false;
auto initBlock =
createInitRegion(builder, loc, var, varName, bounds, varType, needsFree);
- if (!initBlock) {
+ if (!initBlock)
return std::nullopt;
- }
// Only create destroy region if the allocation needs deallocation
std::unique_ptr<Block> destroyBlock;
@@ -1239,9 +1231,8 @@ PrivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
Value allocRes = yieldOp.getOperand(0);
destroyBlock = createDestroyRegion(builder, loc, allocRes, bounds, varType);
- if (!destroyBlock) {
+ if (!destroyBlock)
return std::nullopt;
- }
}
// Now create the recipe operation at the original insertion point and attach
@@ -1298,10 +1289,9 @@ FirstprivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
bool isMappable = isa<MappableType>(varType);
bool isPointerLike = isa<PointerLikeType>(varType);
- if (!isMappable && !isPointerLike) {
- // Unsupported type
+ // Unsupported type
+ if (!isMappable && !isPointerLike)
return std::nullopt;
- }
// Create init, copy, and destroy blocks using shared helpers
OpBuilder::InsertionGuard guard(builder);
@@ -1312,14 +1302,12 @@ FirstprivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
bool needsFree = false;
auto initBlock =
createInitRegion(builder, loc, var, varName, bounds, varType, needsFree);
- if (!initBlock) {
+ if (!initBlock)
return std::nullopt;
- }
auto copyBlock = createCopyRegion(builder, loc, bounds, varType);
- if (!copyBlock) {
+ if (!copyBlock)
return std::nullopt;
- }
// Only create destroy region if the allocation needs deallocation
std::unique_ptr<Block> destroyBlock;
@@ -1329,9 +1317,8 @@ FirstprivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
Value allocRes = yieldOp.getOperand(0);
destroyBlock = createDestroyRegion(builder, loc, allocRes, bounds, varType);
- if (!destroyBlock) {
+ if (!destroyBlock)
return std::nullopt;
- }
}
// Now create the recipe operation at the original insertion point and attach
>From 417e58692dd3f6149b5e5287027669d5cf078336 Mon Sep 17 00:00:00 2001
From: Razvan Lupusoru <rlupusoru at nvidia.com>
Date: Mon, 13 Oct 2025 16:09:44 -0700
Subject: [PATCH 4/6] Add handling for duplicate symbols
---
mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index c568dece1fb0a..634bb0a9771bd 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -17,6 +17,7 @@
#include "mlir/IR/DialectImplementation.h"
#include "mlir/IR/Matchers.h"
#include "mlir/IR/OpImplementation.h"
+#include "mlir/IR/SymbolTable.h"
#include "mlir/Support/LLVM.h"
#include "mlir/Transforms/DialectConversion.h"
#include "llvm/ADT/SmallSet.h"
@@ -1201,6 +1202,14 @@ PrivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
StringRef recipeName, Value var,
StringRef varName, ValueRange bounds) {
+ // Check if a symbol with this name already exists in the symbol table.
+ // If it does - don't assume that we can just reuse the existing one.
+ Operation *parentOp = builder.getInsertionBlock()->getParentOp();
+ if (parentOp &&
+ SymbolTable::lookupNearestSymbolFrom(
+ parentOp, mlir::StringAttr::get(builder.getContext(), varName)))
+ return std::nullopt;
+
Type varType = var.getType();
// First, validate that we can handle this variable type
@@ -1283,6 +1292,14 @@ FirstprivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
StringRef recipeName, Value var,
StringRef varName, ValueRange bounds) {
+ // Check if a symbol with this name already exists in the symbol table.
+ // If it does - don't assume that we can just reuse the existing one.
+ Operation *parentOp = builder.getInsertionBlock()->getParentOp();
+ if (parentOp &&
+ SymbolTable::lookupNearestSymbolFrom(
+ parentOp, mlir::StringAttr::get(builder.getContext(), varName)))
+ return std::nullopt;
+
Type varType = var.getType();
// First, validate that we can handle this variable type
>From 2d4e9ba53b9a09431a2c73bb40aac689f323a895 Mon Sep 17 00:00:00 2001
From: Razvan Lupusoru <rlupusoru at nvidia.com>
Date: Tue, 14 Oct 2025 09:44:01 -0700
Subject: [PATCH 5/6] Use varType instead of var in createAndPopulate
---
.../mlir/Dialect/OpenACC/OpenACCOps.td | 4 +--
mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp | 32 ++++++++-----------
.../Dialect/OpenACC/TestRecipePopulate.cpp | 6 ++--
3 files changed, 19 insertions(+), 23 deletions(-)
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index c22c0dec6a2a1..9a01c2b93118a 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -1328,7 +1328,7 @@ def OpenACC_PrivateRecipeOp
::mlir::OpBuilder &builder,
::mlir::Location loc,
::llvm::StringRef recipeName,
- ::mlir::Value var,
+ ::mlir::Type varType,
::llvm::StringRef varName = "",
::mlir::ValueRange bounds = {});
}];
@@ -1438,7 +1438,7 @@ def OpenACC_FirstprivateRecipeOp
::mlir::OpBuilder &builder,
::mlir::Location loc,
::llvm::StringRef recipeName,
- ::mlir::Value var,
+ ::mlir::Type varType,
::llvm::StringRef varName = "",
::mlir::ValueRange bounds = {});
}];
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index 634bb0a9771bd..54b26750dbe7b 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -1019,8 +1019,8 @@ struct RemoveConstantIfConditionWithRegion : public OpRewritePattern<OpTy> {
/// Returns the init block on success, or nullptr on failure.
/// Sets needsFree to indicate if the allocated memory requires deallocation.
static std::unique_ptr<Block> createInitRegion(OpBuilder &builder, Location loc,
- Value var, StringRef varName,
- ValueRange bounds, Type varType,
+ Type varType, StringRef varName,
+ ValueRange bounds,
bool &needsFree) {
// Create init block with arguments: original value + bounds
SmallVector<Type> argTypes{varType};
@@ -1070,8 +1070,8 @@ static std::unique_ptr<Block> createInitRegion(OpBuilder &builder, Location loc,
/// Returns the copy block on success, or nullptr on failure.
/// TODO: Handle MappableType - it does not yet have a copy API.
static std::unique_ptr<Block> createCopyRegion(OpBuilder &builder, Location loc,
- ValueRange bounds,
- Type varType) {
+ Type varType,
+ ValueRange bounds) {
// Create copy block with arguments: original value + privatized value +
// bounds
SmallVector<Type> copyArgTypes{varType, varType};
@@ -1114,9 +1114,9 @@ static std::unique_ptr<Block> createCopyRegion(OpBuilder &builder, Location loc,
/// Create and populate a destroy region for privatization recipes.
/// Returns the destroy block on success, or nullptr if not needed.
static std::unique_ptr<Block> createDestroyRegion(OpBuilder &builder,
- Location loc, Value allocRes,
- ValueRange bounds,
- Type varType) {
+ Location loc, Type varType,
+ Value allocRes,
+ ValueRange bounds) {
// Create destroy block with arguments: original value + privatized value +
// bounds
SmallVector<Type> destroyArgTypes{varType, varType};
@@ -1199,7 +1199,7 @@ LogicalResult acc::PrivateRecipeOp::verifyRegions() {
std::optional<PrivateRecipeOp>
PrivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
- StringRef recipeName, Value var,
+ StringRef recipeName, Type varType,
StringRef varName, ValueRange bounds) {
// Check if a symbol with this name already exists in the symbol table.
@@ -1210,8 +1210,6 @@ PrivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
parentOp, mlir::StringAttr::get(builder.getContext(), varName)))
return std::nullopt;
- Type varType = var.getType();
-
// First, validate that we can handle this variable type
bool isMappable = isa<MappableType>(varType);
bool isPointerLike = isa<PointerLikeType>(varType);
@@ -1228,7 +1226,7 @@ PrivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
bool needsFree = false;
auto initBlock =
- createInitRegion(builder, loc, var, varName, bounds, varType, needsFree);
+ createInitRegion(builder, loc, varType, varName, bounds, needsFree);
if (!initBlock)
return std::nullopt;
@@ -1239,7 +1237,7 @@ PrivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
auto yieldOp = cast<acc::YieldOp>(initBlock->getTerminator());
Value allocRes = yieldOp.getOperand(0);
- destroyBlock = createDestroyRegion(builder, loc, allocRes, bounds, varType);
+ destroyBlock = createDestroyRegion(builder, loc, varType, allocRes, bounds);
if (!destroyBlock)
return std::nullopt;
}
@@ -1289,7 +1287,7 @@ LogicalResult acc::FirstprivateRecipeOp::verifyRegions() {
std::optional<FirstprivateRecipeOp>
FirstprivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
- StringRef recipeName, Value var,
+ StringRef recipeName, Type varType,
StringRef varName, ValueRange bounds) {
// Check if a symbol with this name already exists in the symbol table.
@@ -1300,8 +1298,6 @@ FirstprivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
parentOp, mlir::StringAttr::get(builder.getContext(), varName)))
return std::nullopt;
- Type varType = var.getType();
-
// First, validate that we can handle this variable type
bool isMappable = isa<MappableType>(varType);
bool isPointerLike = isa<PointerLikeType>(varType);
@@ -1318,11 +1314,11 @@ FirstprivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
bool needsFree = false;
auto initBlock =
- createInitRegion(builder, loc, var, varName, bounds, varType, needsFree);
+ createInitRegion(builder, loc, varType, varName, bounds, needsFree);
if (!initBlock)
return std::nullopt;
- auto copyBlock = createCopyRegion(builder, loc, bounds, varType);
+ auto copyBlock = createCopyRegion(builder, loc, varType, bounds);
if (!copyBlock)
return std::nullopt;
@@ -1333,7 +1329,7 @@ FirstprivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
auto yieldOp = cast<acc::YieldOp>(initBlock->getTerminator());
Value allocRes = yieldOp.getOperand(0);
- destroyBlock = createDestroyRegion(builder, loc, allocRes, bounds, varType);
+ destroyBlock = createDestroyRegion(builder, loc, varType, allocRes, bounds);
if (!destroyBlock)
return std::nullopt;
}
diff --git a/mlir/test/lib/Dialect/OpenACC/TestRecipePopulate.cpp b/mlir/test/lib/Dialect/OpenACC/TestRecipePopulate.cpp
index d3664b6eebbd4..35f092c2188a6 100644
--- a/mlir/test/lib/Dialect/OpenACC/TestRecipePopulate.cpp
+++ b/mlir/test/lib/Dialect/OpenACC/TestRecipePopulate.cpp
@@ -80,15 +80,15 @@ void TestRecipePopulatePass::runOnOperation() {
ValueRange bounds; // No bounds for memref tests
if (recipeType == "private") {
- auto recipe = PrivateRecipeOp::createAndPopulate(builder, loc, recipeName,
- var, varName, bounds);
+ auto recipe = PrivateRecipeOp::createAndPopulate(
+ builder, loc, recipeName, var.getType(), varName, bounds);
if (!recipe) {
op->emitError("Failed to create private recipe for ") << varName;
}
} else if (recipeType == "firstprivate") {
auto recipe = FirstprivateRecipeOp::createAndPopulate(
- builder, loc, recipeName, var, varName, bounds);
+ builder, loc, recipeName, var.getType(), varName, bounds);
if (!recipe) {
op->emitError("Failed to create firstprivate recipe for ") << varName;
>From 493b72a2e6b0fde4db7810051a61fd36b4ac4734 Mon Sep 17 00:00:00 2001
From: Razvan Lupusoru <rlupusoru at nvidia.com>
Date: Tue, 14 Oct 2025 09:53:39 -0700
Subject: [PATCH 6/6] Remove symbol table lookup
---
.../include/mlir/Dialect/OpenACC/OpenACCOps.td | 8 ++++++--
mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp | 18 ------------------
2 files changed, 6 insertions(+), 20 deletions(-)
diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index 9a01c2b93118a..fecf81b4f99ea 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -1323,7 +1323,9 @@ def OpenACC_PrivateRecipeOp
/// PointerLikeType interface. If a type implements both, the MappableType
/// API will be preferred. Returns std::nullopt if the recipe cannot be
/// created or populated. The builder's current insertion point will be used
- /// and it must be a valid place for this operation to be inserted.
+ /// and it must be a valid place for this operation to be inserted. The
+ /// `recipeName` must be a unique name to prevent "redefinition of symbol"
+ /// IR errors.
static std::optional<PrivateRecipeOp> createAndPopulate(
::mlir::OpBuilder &builder,
::mlir::Location loc,
@@ -1433,7 +1435,9 @@ def OpenACC_FirstprivateRecipeOp
/// PointerLikeType interface. If a type implements both, the MappableType
/// API will be preferred. Returns std::nullopt if the recipe cannot be
/// created or populated. The builder's current insertion point will be used
- /// and it must be a valid place for this operation to be inserted.
+ /// and it must be a valid place for this operation to be inserted. The
+ /// `recipeName` must be a unique name to prevent "redefinition of symbol"
+ /// IR errors.
static std::optional<FirstprivateRecipeOp> createAndPopulate(
::mlir::OpBuilder &builder,
::mlir::Location loc,
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index 54b26750dbe7b..9ce6fbec52395 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -1201,15 +1201,6 @@ std::optional<PrivateRecipeOp>
PrivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
StringRef recipeName, Type varType,
StringRef varName, ValueRange bounds) {
-
- // Check if a symbol with this name already exists in the symbol table.
- // If it does - don't assume that we can just reuse the existing one.
- Operation *parentOp = builder.getInsertionBlock()->getParentOp();
- if (parentOp &&
- SymbolTable::lookupNearestSymbolFrom(
- parentOp, mlir::StringAttr::get(builder.getContext(), varName)))
- return std::nullopt;
-
// First, validate that we can handle this variable type
bool isMappable = isa<MappableType>(varType);
bool isPointerLike = isa<PointerLikeType>(varType);
@@ -1289,15 +1280,6 @@ std::optional<FirstprivateRecipeOp>
FirstprivateRecipeOp::createAndPopulate(OpBuilder &builder, Location loc,
StringRef recipeName, Type varType,
StringRef varName, ValueRange bounds) {
-
- // Check if a symbol with this name already exists in the symbol table.
- // If it does - don't assume that we can just reuse the existing one.
- Operation *parentOp = builder.getInsertionBlock()->getParentOp();
- if (parentOp &&
- SymbolTable::lookupNearestSymbolFrom(
- parentOp, mlir::StringAttr::get(builder.getContext(), varName)))
- return std::nullopt;
-
// First, validate that we can handle this variable type
bool isMappable = isa<MappableType>(varType);
bool isPointerLike = isa<PointerLikeType>(varType);
More information about the Mlir-commits
mailing list