From d91508e87c5f470126122420559108e0bc5a2ad3 Mon Sep 17 00:00:00 2001 From: Razvan Lupusoru Date: Thu, 16 Oct 2025 13:45:25 -0700 Subject: [PATCH 1/2] [mlir][acc] Ensure genAllocate uses provided variable name The genAllocate API was documented to have the `varName` argument as optional. However, when it is provided, it becomes unexpected if the implementation does not use it. Since not all dialects have a way to store variable names, add one in the acc dialect and use it to store names of memref variables. This updates the API documentation, implementation of genAllocate for memref, and IR testing. --- mlir/include/mlir/Dialect/OpenACC/OpenACC.h | 4 ++++ .../mlir/Dialect/OpenACC/OpenACCOps.td | 7 +++++++ .../Dialect/OpenACC/OpenACCTypeInterfaces.td | 14 +++++++++---- mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp | 20 ++++++++++++++++--- .../OpenACC/pointer-like-interface-alloc.mlir | 4 ++-- .../OpenACC/recipe-populate-firstprivate.mlir | 10 +++++----- .../OpenACC/recipe-populate-private.mlir | 10 +++++----- 7 files changed, 50 insertions(+), 19 deletions(-) diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACC.h b/mlir/include/mlir/Dialect/OpenACC/OpenACC.h index 8f87235fcd237..b8aa49752d0a9 100644 --- a/mlir/include/mlir/Dialect/OpenACC/OpenACC.h +++ b/mlir/include/mlir/Dialect/OpenACC/OpenACC.h @@ -183,6 +183,10 @@ static constexpr StringLiteral getRoutineInfoAttrName() { return StringLiteral("acc.routine_info"); } +static constexpr StringLiteral getVarNameAttrName() { + return VarNameAttr::name; +} + static constexpr StringLiteral getCombinedConstructsAttrName() { return CombinedConstructsTypeAttr::name; } diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td index fecf81b4f99ea..1eaa21b46554c 100644 --- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td +++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td @@ -415,6 +415,13 @@ def OpenACC_ConstructResource : Resource<"::mlir::acc::ConstructResource">; // Define a resource for the OpenACC current device setting. def OpenACC_CurrentDeviceIdResource : Resource<"::mlir::acc::CurrentDeviceIdResource">; +// Attribute for saving variable names - this can be attached to non-acc-dialect +// operations in order to ensure the name is preserved. +def OpenACC_VarNameAttr : OpenACC_Attr<"VarName", "var_name"> { + let parameters = (ins StringRefParameter<"">:$name); + let assemblyFormat = "`<` $name `>`"; +} + // Used for data specification in data clauses (2.7.1). // Either (or both) extent and upperbound must be specified. def OpenACC_DataBoundsOp : OpenACC_Op<"bounds", diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCTypeInterfaces.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCTypeInterfaces.td index 6736bc861fece..93e9e3d0689f7 100644 --- a/mlir/include/mlir/Dialect/OpenACC/OpenACCTypeInterfaces.td +++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCTypeInterfaces.td @@ -73,12 +73,18 @@ def OpenACC_PointerLikeTypeInterface : TypeInterface<"PointerLikeType"> { InterfaceMethod< /*description=*/[{ Generates allocation operations for the pointer-like type. It will create - an allocate that produces memory space for an instance of the current type. + an allocate operation that produces memory space for an instance of the + current type. The `varName` parameter is optional and can be used to provide a name - for the allocated variable. 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. + for the allocated variable. When provided, it must be used by the + implementation; and if the implementing dialect does not have its own + way to save it, the discardable `acc.var_name` attribute from the acc + dialect will be used. + + 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. The `originalVar` parameter is optional but enables support for dynamic types (e.g., dynamic memrefs). When provided, implementations can extract diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp index 642ced94e3489..508708f11ca92 100644 --- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp +++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp @@ -40,6 +40,17 @@ static bool isScalarLikeType(Type type) { return type.isIntOrIndexOrFloat() || isa(type); } +/// Helper function to attach the `VarName` attribute to an operation +/// if a variable name is provided. +static void attachVarNameAttr(Operation *op, OpBuilder &builder, + StringRef varName) { + if (!varName.empty()) { + auto varNameAttr = + acc::VarNameAttr::get(builder.getContext(), varName); + op->setAttr(acc::getVarNameAttrName(), varNameAttr); + } +} + struct MemRefPointerLikeModel : public PointerLikeType::ExternalModel { @@ -83,7 +94,9 @@ struct MemRefPointerLikeModel // then we can generate an alloca operation. if (memrefTy.hasStaticShape()) { needsFree = false; // alloca doesn't need deallocation - return memref::AllocaOp::create(builder, loc, memrefTy).getResult(); + auto allocaOp = memref::AllocaOp::create(builder, loc, memrefTy); + attachVarNameAttr(allocaOp, builder, varName); + return allocaOp.getResult(); } // For dynamic memrefs, extract sizes from the original variable if @@ -103,8 +116,9 @@ struct MemRefPointerLikeModel // Static dimensions are handled automatically by AllocOp } needsFree = true; // alloc needs deallocation - return memref::AllocOp::create(builder, loc, memrefTy, dynamicSizes) - .getResult(); + auto allocOp = memref::AllocOp::create(builder, loc, memrefTy, dynamicSizes); + attachVarNameAttr(allocOp, builder, varName); + return allocOp.getResult(); } // TODO: Unranked not yet supported. diff --git a/mlir/test/Dialect/OpenACC/pointer-like-interface-alloc.mlir b/mlir/test/Dialect/OpenACC/pointer-like-interface-alloc.mlir index 603ace85072ac..3d4bec7f4637e 100644 --- a/mlir/test/Dialect/OpenACC/pointer-like-interface-alloc.mlir +++ b/mlir/test/Dialect/OpenACC/pointer-like-interface-alloc.mlir @@ -3,7 +3,7 @@ func.func @test_static_memref_alloc() { %0 = memref.alloca() {test.ptr} : memref<10x20xf32> // CHECK: Successfully generated alloc for operation: %[[ORIG:.*]] = memref.alloca() {test.ptr} : memref<10x20xf32> - // CHECK: Generated: %{{.*}} = memref.alloca() : memref<10x20xf32> + // CHECK: Generated: %{{.*}} = memref.alloca() {acc.var_name = #acc.var_name<"test_alloc">} : memref<10x20xf32> return } @@ -19,6 +19,6 @@ func.func @test_dynamic_memref_alloc() { // CHECK: Generated: %[[DIM0:.*]] = memref.dim %[[ORIG]], %[[C0]] : memref // CHECK: Generated: %[[C1:.*]] = arith.constant 1 : index // CHECK: Generated: %[[DIM1:.*]] = memref.dim %[[ORIG]], %[[C1]] : memref - // CHECK: Generated: %{{.*}} = memref.alloc(%[[DIM0]], %[[DIM1]]) : memref + // CHECK: Generated: %{{.*}} = memref.alloc(%[[DIM0]], %[[DIM1]]) {acc.var_name = #acc.var_name<"test_alloc">} : memref return } diff --git a/mlir/test/Dialect/OpenACC/recipe-populate-firstprivate.mlir b/mlir/test/Dialect/OpenACC/recipe-populate-firstprivate.mlir index 35355c6e06164..8846c9e76a27a 100644 --- a/mlir/test/Dialect/OpenACC/recipe-populate-firstprivate.mlir +++ b/mlir/test/Dialect/OpenACC/recipe-populate-firstprivate.mlir @@ -2,7 +2,7 @@ // CHECK: acc.firstprivate.recipe @firstprivate_scalar : memref init { // CHECK: ^bb0(%{{.*}}: memref): -// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref +// CHECK: %[[ALLOC:.*]] = memref.alloca() {acc.var_name = #acc.var_name<"scalar">} : memref // CHECK: acc.yield %[[ALLOC]] : memref // CHECK: } copy { // CHECK: ^bb0(%[[SRC:.*]]: memref, %[[DST:.*]]: memref): @@ -20,7 +20,7 @@ func.func @test_scalar() { // CHECK: acc.firstprivate.recipe @firstprivate_static_2d : memref<10x20xf32> init { // CHECK: ^bb0(%{{.*}}: memref<10x20xf32>): -// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref<10x20xf32> +// CHECK: %[[ALLOC:.*]] = memref.alloca() {acc.var_name = #acc.var_name<"static_2d">} : memref<10x20xf32> // CHECK: acc.yield %[[ALLOC]] : memref<10x20xf32> // CHECK: } copy { // CHECK: ^bb0(%[[SRC:.*]]: memref<10x20xf32>, %[[DST:.*]]: memref<10x20xf32>): @@ -42,7 +42,7 @@ func.func @test_static_2d() { // CHECK: %[[DIM0:.*]] = memref.dim %[[ARG]], %[[C0]] : memref // CHECK: %[[C1:.*]] = arith.constant 1 : index // CHECK: %[[DIM1:.*]] = memref.dim %[[ARG]], %[[C1]] : memref -// CHECK: %[[ALLOC:.*]] = memref.alloc(%[[DIM0]], %[[DIM1]]) : memref +// CHECK: %[[ALLOC:.*]] = memref.alloc(%[[DIM0]], %[[DIM1]]) {acc.var_name = #acc.var_name<"dynamic_2d">} : memref // CHECK: acc.yield %[[ALLOC]] : memref // CHECK: } copy { // CHECK: ^bb0(%[[SRC:.*]]: memref, %[[DST:.*]]: memref): @@ -65,7 +65,7 @@ func.func @test_dynamic_2d(%arg0: index, %arg1: index) { // 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: %[[ALLOC:.*]] = memref.alloc(%[[DIM1]]) {acc.var_name = #acc.var_name<"mixed_dims">} : memref<10x?xf32> // CHECK: acc.yield %[[ALLOC]] : memref<10x?xf32> // CHECK: } copy { // CHECK: ^bb0(%[[SRC:.*]]: memref<10x?xf32>, %[[DST:.*]]: memref<10x?xf32>): @@ -86,7 +86,7 @@ func.func @test_mixed_dims(%arg0: index) { // CHECK: acc.firstprivate.recipe @firstprivate_scalar_int : memref init { // CHECK: ^bb0(%{{.*}}: memref): -// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref +// CHECK: %[[ALLOC:.*]] = memref.alloca() {acc.var_name = #acc.var_name<"scalar_int">} : memref // CHECK: acc.yield %[[ALLOC]] : memref // CHECK: } copy { // CHECK: ^bb0(%[[SRC:.*]]: memref, %[[DST:.*]]: memref): diff --git a/mlir/test/Dialect/OpenACC/recipe-populate-private.mlir b/mlir/test/Dialect/OpenACC/recipe-populate-private.mlir index 8403ee80a7bc6..3d5a91839da0d 100644 --- a/mlir/test/Dialect/OpenACC/recipe-populate-private.mlir +++ b/mlir/test/Dialect/OpenACC/recipe-populate-private.mlir @@ -2,7 +2,7 @@ // CHECK: acc.private.recipe @private_scalar : memref init { // CHECK: ^bb0(%{{.*}}: memref): -// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref +// CHECK: %[[ALLOC:.*]] = memref.alloca() {acc.var_name = #acc.var_name<"scalar">} : memref // CHECK: acc.yield %[[ALLOC]] : memref // CHECK: } // CHECK-NOT: destroy @@ -16,7 +16,7 @@ func.func @test_scalar() { // CHECK: acc.private.recipe @private_static_2d : memref<10x20xf32> init { // CHECK: ^bb0(%{{.*}}: memref<10x20xf32>): -// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref<10x20xf32> +// CHECK: %[[ALLOC:.*]] = memref.alloca() {acc.var_name = #acc.var_name<"static_2d">} : memref<10x20xf32> // CHECK: acc.yield %[[ALLOC]] : memref<10x20xf32> // CHECK: } // CHECK-NOT: destroy @@ -34,7 +34,7 @@ func.func @test_static_2d() { // CHECK: %[[DIM0:.*]] = memref.dim %[[ARG]], %[[C0]] : memref // CHECK: %[[C1:.*]] = arith.constant 1 : index // CHECK: %[[DIM1:.*]] = memref.dim %[[ARG]], %[[C1]] : memref -// CHECK: %[[ALLOC:.*]] = memref.alloc(%[[DIM0]], %[[DIM1]]) : memref +// CHECK: %[[ALLOC:.*]] = memref.alloc(%[[DIM0]], %[[DIM1]]) {acc.var_name = #acc.var_name<"dynamic_2d">} : memref // CHECK: acc.yield %[[ALLOC]] : memref // CHECK: } destroy { // CHECK: ^bb0(%{{.*}}: memref, %[[VAL:.*]]: memref): @@ -53,7 +53,7 @@ func.func @test_dynamic_2d(%arg0: index, %arg1: index) { // 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: %[[ALLOC:.*]] = memref.alloc(%[[DIM1]]) {acc.var_name = #acc.var_name<"mixed_dims">} : memref<10x?xf32> // CHECK: acc.yield %[[ALLOC]] : memref<10x?xf32> // CHECK: } destroy { // CHECK: ^bb0(%{{.*}}: memref<10x?xf32>, %[[VAL:.*]]: memref<10x?xf32>): @@ -70,7 +70,7 @@ func.func @test_mixed_dims(%arg0: index) { // CHECK: acc.private.recipe @private_scalar_int : memref init { // CHECK: ^bb0(%{{.*}}: memref): -// CHECK: %[[ALLOC:.*]] = memref.alloca() : memref +// CHECK: %[[ALLOC:.*]] = memref.alloca() {acc.var_name = #acc.var_name<"scalar_int">} : memref // CHECK: acc.yield %[[ALLOC]] : memref // CHECK: } // CHECK-NOT: destroy From d4cba2f2eacceb28ff5274b0c4ed6e8ed995144f Mon Sep 17 00:00:00 2001 From: Razvan Lupusoru Date: Thu, 16 Oct 2025 13:49:50 -0700 Subject: [PATCH 2/2] Fix formatting --- mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp index 508708f11ca92..90cbbd86dc002 100644 --- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp +++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp @@ -43,10 +43,9 @@ static bool isScalarLikeType(Type type) { /// Helper function to attach the `VarName` attribute to an operation /// if a variable name is provided. static void attachVarNameAttr(Operation *op, OpBuilder &builder, - StringRef varName) { + StringRef varName) { if (!varName.empty()) { - auto varNameAttr = - acc::VarNameAttr::get(builder.getContext(), varName); + auto varNameAttr = acc::VarNameAttr::get(builder.getContext(), varName); op->setAttr(acc::getVarNameAttrName(), varNameAttr); } } @@ -116,8 +115,9 @@ struct MemRefPointerLikeModel // Static dimensions are handled automatically by AllocOp } needsFree = true; // alloc needs deallocation - auto allocOp = memref::AllocOp::create(builder, loc, memrefTy, dynamicSizes); - attachVarNameAttr(allocOp, builder, varName); + auto allocOp = + memref::AllocOp::create(builder, loc, memrefTy, dynamicSizes); + attachVarNameAttr(allocOp, builder, varName); return allocOp.getResult(); }