Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -1316,6 +1316,24 @@ 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. 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,
::llvm::StringRef recipeName,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it up to the caller to ensure recipe with that name have not been created yet/what happens if it already exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question! The code wasn't handling this case previously.

Contrary to my initial belief, MLIR does not automatically make symbol names unique. Instead, it allows construction of IR that will later fail verification with a "redefinition of symbol" error.

I've updated createAndPopulate to return std::nullopt when a symbol with the requested name already exists. This indicates to the caller that the operation cannot be created.

I considered two alternative approaches:

  1. Return the existing recipe - I rejected this because we cannot guarantee that the existing recipe is semantically equivalent to what the caller intends to create without additional validation.
  2. Auto-generate a unique name - I rejected this because it would silently create a recipe with a different name than what the caller requested, which could lead to unexpected behavior.

By returning std::nullopt, the caller receives clear feedback that the requested name conflicts with an existing symbol and can handle it appropriately (e.g., use a different name, check if the existing recipe is suitable, etc.).

::mlir::Type varType,
::llvm::StringRef varName = "",
::mlir::ValueRange bounds = {});
}];
}

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -1410,6 +1428,24 @@ 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. 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,
::llvm::StringRef recipeName,
::mlir::Type varType,
::llvm::StringRef varName = "",
::mlir::ValueRange bounds = {});
}];
}

//===----------------------------------------------------------------------===//
Expand Down
44 changes: 32 additions & 12 deletions mlir/include/mlir/Dialect/OpenACC/OpenACCTypeInterfaces.td
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -94,31 +102,43 @@ 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 {};
}]
>,
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=*/[{
Expand Down
Loading