From 3d6c19ff10ee906947a657fcc5dfebd30e031c30 Mon Sep 17 00:00:00 2001 From: Razvan Lupusoru Date: Wed, 15 Oct 2025 16:45:45 -0700 Subject: [PATCH 1/2] [flang][acc] Implement PointerLikeType API gen[Allocate/Free/Copy] Implements genAllocate, genFree, and genCopy for FIR pointer types (fir.ref, fir.ptr, fir.heap, fir.llvm_ptr) in the OpenACC PointerLikeType interface. - genAllocate: Uses fir.alloca for stack types, fir.allocmem for heap types. Returns null for dynamic/unknown types (unlimited polymorphic, dynamic arrays, dynamic character lengths). - genFree: Generates fir.freemem for heap allocations. - genCopy: Uses fir.load+fir.store for trivial types (scalars), hlfir.assign for non-trivial types (arrays, derived types, characters). Returns false for unsupported dynamic types. Adds comprehensive MLIR tests covering various FIR types and edge cases. --- .../Support/FIROpenACCTypeInterfaces.h | 14 ++ .../Support/FIROpenACCTypeInterfaces.cpp | 222 ++++++++++++++++++ .../OpenACC/pointer-like-interface-alloc.mlir | 122 ++++++++++ .../OpenACC/pointer-like-interface-copy.mlir | 120 ++++++++++ .../OpenACC/pointer-like-interface-free.mlir | 94 ++++++++ flang/tools/fir-opt/CMakeLists.txt | 1 + flang/tools/fir-opt/fir-opt.cpp | 6 +- 7 files changed, 578 insertions(+), 1 deletion(-) create mode 100644 flang/test/Fir/OpenACC/pointer-like-interface-alloc.mlir create mode 100644 flang/test/Fir/OpenACC/pointer-like-interface-copy.mlir create mode 100644 flang/test/Fir/OpenACC/pointer-like-interface-free.mlir diff --git a/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.h b/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.h index 408f0392e4271..4817ed933ba06 100644 --- a/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.h +++ b/flang/include/flang/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.h @@ -29,6 +29,20 @@ struct OpenACCPointerLikeModel getPointeeTypeCategory(mlir::Type pointer, mlir::TypedValue varPtr, mlir::Type varType) const; + + mlir::Value genAllocate(mlir::Type pointer, mlir::OpBuilder &builder, + mlir::Location loc, llvm::StringRef varName, + mlir::Type varType, mlir::Value originalVar, + bool &needsFree) const; + + bool genFree(mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + mlir::TypedValue varToFree, + mlir::Value allocRes, mlir::Type varType) const; + + bool genCopy(mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + mlir::TypedValue destination, + mlir::TypedValue source, + mlir::Type varType) const; }; template diff --git a/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp b/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp index 9bf10b53108c0..c543ecbbef2b0 100644 --- a/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp +++ b/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp @@ -751,4 +751,226 @@ template bool OpenACCMappableModel::generatePrivateDestroy( mlir::Type type, mlir::OpBuilder &builder, mlir::Location loc, mlir::Value privatized) const; +template +mlir::Value OpenACCPointerLikeModel::genAllocate( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + llvm::StringRef varName, mlir::Type varType, mlir::Value originalVar, + bool &needsFree) const { + + // Get the element type from the pointer type + mlir::Type eleTy = mlir::cast(pointer).getElementType(); + + // Unlimited polymorphic (class(*)) cannot be handled - size unknown + if (fir::isUnlimitedPolymorphicType(eleTy)) + return {}; + + // Character types with dynamic length cannot be handled without a descriptor. + if (auto charTy = mlir::dyn_cast(eleTy)) + if (charTy.hasDynamicLen()) + return {}; + + // Return null for dynamic or unknown shape arrays because the size of the + // allocation cannot be determined simply from the type. + if (auto seqTy = mlir::dyn_cast(eleTy)) + if (seqTy.hasDynamicExtents() || seqTy.hasUnknownShape()) + return {}; + + // Use heap allocation for fir.heap, stack allocation for others (fir.ref, + // fir.ptr, fir.llvm_ptr). For fir.ptr, which is supposed to represent a + // Fortran pointer type, it feels a bit odd to "allocate" since it is meant + // to point to an existing entity - but one can imagine where a pointee is + // privatized - thus it makes sense to issue an allocate. + mlir::Value allocation; + if (std::is_same_v) { + needsFree = true; + allocation = fir::AllocMemOp::create(builder, loc, eleTy); + } else { + needsFree = false; + allocation = fir::AllocaOp::create(builder, loc, eleTy); + } + + // Convert to the requested pointer type if needed. + // This means converting from a fir.ref to either a fir.llvm_ptr or a fir.ptr. + // fir.heap is already correct type in this case. + if (allocation.getType() != pointer) { + assert(!(std::is_same_v) && + "fir.heap is already correct type because of allocmem"); + return fir::ConvertOp::create(builder, loc, pointer, allocation); + } + + return allocation; +} + +template mlir::Value OpenACCPointerLikeModel::genAllocate( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + llvm::StringRef varName, mlir::Type varType, mlir::Value originalVar, + bool &needsFree) const; + +template mlir::Value OpenACCPointerLikeModel::genAllocate( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + llvm::StringRef varName, mlir::Type varType, mlir::Value originalVar, + bool &needsFree) const; + +template mlir::Value OpenACCPointerLikeModel::genAllocate( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + llvm::StringRef varName, mlir::Type varType, mlir::Value originalVar, + bool &needsFree) const; + +template mlir::Value OpenACCPointerLikeModel::genAllocate( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + llvm::StringRef varName, mlir::Type varType, mlir::Value originalVar, + bool &needsFree) const; + +static mlir::Value stripCasts(mlir::Value value, bool stripDeclare = true) { + mlir::Value currentValue = value; + + while (currentValue) { + auto *definingOp = currentValue.getDefiningOp(); + if (!definingOp) + break; + + if (auto convertOp = mlir::dyn_cast(definingOp)) { + currentValue = convertOp.getValue(); + continue; + } + + if (auto viewLike = mlir::dyn_cast(definingOp)) { + currentValue = viewLike.getViewSource(); + continue; + } + + if (stripDeclare) { + if (auto declareOp = mlir::dyn_cast(definingOp)) { + currentValue = declareOp.getMemref(); + continue; + } + + if (auto declareOp = mlir::dyn_cast(definingOp)) { + currentValue = declareOp.getMemref(); + continue; + } + } + break; + } + + return currentValue; +} + +template +bool OpenACCPointerLikeModel::genFree( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + mlir::TypedValue varToFree, + mlir::Value allocRes, mlir::Type varType) const { + + // If pointer type is HeapType, assume it's a heap allocation + if (std::is_same_v) { + fir::FreeMemOp::create(builder, loc, varToFree); + return true; + } + + // Use allocRes if provided to determine the allocation type + mlir::Value valueToInspect = allocRes ? allocRes : varToFree; + + // Strip casts and declare operations to find the original allocation + mlir::Value strippedValue = stripCasts(valueToInspect); + mlir::Operation *originalAlloc = strippedValue.getDefiningOp(); + + // If we found an AllocMemOp (heap allocation), free it + if (mlir::isa_and_nonnull(originalAlloc)) { + mlir::Value toFree = varToFree; + if (!mlir::isa(valueToInspect.getType())) + toFree = fir::ConvertOp::create( + builder, loc, + fir::HeapType::get(varToFree.getType().getElementType()), toFree); + fir::FreeMemOp::create(builder, loc, toFree); + return true; + } + + // If we found an AllocaOp (stack allocation), no deallocation needed + if (mlir::isa_and_nonnull(originalAlloc)) + return true; + + // Unable to determine allocation type + return false; +} + +template bool OpenACCPointerLikeModel::genFree( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + mlir::TypedValue varToFree, + mlir::Value allocRes, mlir::Type varType) const; + +template bool OpenACCPointerLikeModel::genFree( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + mlir::TypedValue varToFree, + mlir::Value allocRes, mlir::Type varType) const; + +template bool OpenACCPointerLikeModel::genFree( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + mlir::TypedValue varToFree, + mlir::Value allocRes, mlir::Type varType) const; + +template bool OpenACCPointerLikeModel::genFree( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + mlir::TypedValue varToFree, + mlir::Value allocRes, mlir::Type varType) const; + +template +bool OpenACCPointerLikeModel::genCopy( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + mlir::TypedValue destination, + mlir::TypedValue source, + mlir::Type varType) const { + + // Check that source and destination types match + if (source.getType() != destination.getType()) + return false; + + mlir::Type eleTy = mlir::cast(pointer).getElementType(); + + // Unlimited polymorphic (class(*)) cannot be handled because source and + // destination types are not known. + if (fir::isUnlimitedPolymorphicType(eleTy)) + return false; + + // Dynamic lengths without descriptor do not have size information. + if (auto charTy = mlir::dyn_cast(eleTy)) + if (charTy.hasDynamicLen()) + return false; + if (auto seqTy = mlir::dyn_cast(eleTy)) + if (seqTy.hasDynamicExtents() || seqTy.hasUnknownShape()) + return false; + + if (fir::isa_trivial(eleTy)) { + auto loadVal = fir::LoadOp::create(builder, loc, source); + fir::StoreOp::create(builder, loc, loadVal, destination); + } else { + hlfir::AssignOp::create(builder, loc, source, destination); + } + return true; +} + +template bool OpenACCPointerLikeModel::genCopy( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + mlir::TypedValue destination, + mlir::TypedValue source, + mlir::Type varType) const; + +template bool OpenACCPointerLikeModel::genCopy( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + mlir::TypedValue destination, + mlir::TypedValue source, + mlir::Type varType) const; + +template bool OpenACCPointerLikeModel::genCopy( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + mlir::TypedValue destination, + mlir::TypedValue source, + mlir::Type varType) const; + +template bool OpenACCPointerLikeModel::genCopy( + mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc, + mlir::TypedValue destination, + mlir::TypedValue source, + mlir::Type varType) const; + } // namespace fir::acc diff --git a/flang/test/Fir/OpenACC/pointer-like-interface-alloc.mlir b/flang/test/Fir/OpenACC/pointer-like-interface-alloc.mlir new file mode 100644 index 0000000000000..0da360a26b3e6 --- /dev/null +++ b/flang/test/Fir/OpenACC/pointer-like-interface-alloc.mlir @@ -0,0 +1,122 @@ +// RUN: fir-opt %s --split-input-file --pass-pipeline="builtin.module(func.func(test-acc-pointer-like-interface{test-mode=alloc}))" 2>&1 | FileCheck %s + +// The tests here use a synthetic hlfir.declare in order to ensure that the hlfir dialect is +// loaded. This is required because the pass used is part of OpenACC test passes outside of +// flang and the APIs being test may generate hlfir even when it does not appear. + +func.func @test_ref_scalar_alloc() { + %0 = fir.alloca f32 {test.ptr} + %1:2 = hlfir.declare %0 {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated alloc for operation: %{{.*}} = fir.alloca f32 {test.ptr} + // CHECK: Generated: %{{.*}} = fir.alloca f32 + return +} + +// ----- + +func.func @test_ref_static_array_alloc() { + %0 = fir.alloca !fir.array<10x20xf32> {test.ptr} + %var = fir.alloca f32 + %1:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated alloc for operation: %{{.*}} = fir.alloca !fir.array<10x20xf32> {test.ptr} + // CHECK: Generated: %{{.*}} = fir.alloca !fir.array<10x20xf32> + return +} + +// ----- + +func.func @test_ref_derived_type_alloc() { + %0 = fir.alloca !fir.type<_QTt{i:i32}> {test.ptr} + %var = fir.alloca f32 + %1:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated alloc for operation: %{{.*}} = fir.alloca !fir.type<_QTt{i:i32}> {test.ptr} + // CHECK: Generated: %{{.*}} = fir.alloca !fir.type<_QTt{i:i32}> + return +} + +// ----- + +func.func @test_heap_scalar_alloc() { + %0 = fir.allocmem f32 {test.ptr} + %var = fir.alloca f32 + %1:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated alloc for operation: %{{.*}} = fir.allocmem f32 {test.ptr} + // CHECK: Generated: %{{.*}} = fir.allocmem f32 + return +} + +// ----- + +func.func @test_heap_static_array_alloc() { + %0 = fir.allocmem !fir.array<10x20xf32> {test.ptr} + %var = fir.alloca f32 + %1:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated alloc for operation: %{{.*}} = fir.allocmem !fir.array<10x20xf32> {test.ptr} + // CHECK: Generated: %{{.*}} = fir.allocmem !fir.array<10x20xf32> + return +} + +// ----- + +func.func @test_ptr_scalar_alloc() { + %0 = fir.alloca f32 + %1 = fir.convert %0 {test.ptr} : (!fir.ref) -> !fir.ptr + %2:2 = hlfir.declare %0 {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated alloc for operation + // CHECK: Generated: %{{.*}} = fir.alloca f32 + // CHECK: Generated: %{{.*}} = fir.convert %{{.*}} : (!fir.ref) -> !fir.ptr + return +} + +// ----- + +func.func @test_llvm_ptr_scalar_alloc() { + %0 = fir.alloca f32 + %1 = fir.convert %0 {test.ptr} : (!fir.ref) -> !fir.llvm_ptr + %2:2 = hlfir.declare %0 {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated alloc for operation + // CHECK: Generated: %{{.*}} = fir.alloca f32 + // CHECK: Generated: %{{.*}} = fir.convert %{{.*}} : (!fir.ref) -> !fir.llvm_ptr + return +} + +// ----- + +func.func @test_dynamic_array_alloc_fails(%arg0: !fir.ref>) { + %0 = fir.convert %arg0 {test.ptr} : (!fir.ref>) -> !fir.llvm_ptr> + %var = fir.alloca f32 + %1:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Failed to generate alloc for operation: %{{.*}} = fir.convert %{{.*}} {test.ptr} : (!fir.ref>) -> !fir.llvm_ptr> + return +} + +// ----- + +func.func @test_unlimited_polymorphic_alloc_fails() { + %0 = fir.alloca !fir.class {test.ptr} + %var = fir.alloca f32 + %1:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Failed to generate alloc for operation: %{{.*}} = fir.alloca !fir.class {test.ptr} + return +} + +// ----- + +func.func @test_dynamic_char_alloc_fails(%arg0: !fir.ref>) { + %0 = fir.convert %arg0 {test.ptr} : (!fir.ref>) -> !fir.llvm_ptr> + %var = fir.alloca f32 + %1:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Failed to generate alloc for operation: %{{.*}} = fir.convert %{{.*}} {test.ptr} : (!fir.ref>) -> !fir.llvm_ptr> + return +} + +// ----- + +func.func @test_static_char_alloc() { + %0 = fir.alloca !fir.char<1,10> {test.ptr} + %var = fir.alloca f32 + %1:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated alloc for operation: %{{.*}} = fir.alloca !fir.char<1,10> {test.ptr} + // CHECK: Generated: %{{.*}} = fir.alloca !fir.char<1,10> + return +} diff --git a/flang/test/Fir/OpenACC/pointer-like-interface-copy.mlir b/flang/test/Fir/OpenACC/pointer-like-interface-copy.mlir new file mode 100644 index 0000000000000..99fc012cb903e --- /dev/null +++ b/flang/test/Fir/OpenACC/pointer-like-interface-copy.mlir @@ -0,0 +1,120 @@ +// RUN: fir-opt %s --split-input-file --pass-pipeline="builtin.module(func.func(test-acc-pointer-like-interface{test-mode=copy}))" 2>&1 | FileCheck %s + +// The tests here use a synthetic hlfir.declare in order to ensure that the hlfir dialect is +// loaded. This is required because the pass used is part of OpenACC test passes outside of +// flang and the APIs being test may generate hlfir even when it does not appear. + +func.func @test_copy_scalar() { + %src = fir.alloca f32 {test.src_ptr} + %dest = fir.alloca f32 {test.dest_ptr} + %var = fir.alloca f32 + %0:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated copy from source: %{{.*}} = fir.alloca f32 {test.src_ptr} to destination: %{{.*}} = fir.alloca f32 {test.dest_ptr} + // CHECK: Generated: %{{.*}} = fir.load %{{.*}} : !fir.ref + // CHECK: Generated: fir.store %{{.*}} to %{{.*}} : !fir.ref + return +} + +// ----- + +func.func @test_copy_static_array() { + %src = fir.alloca !fir.array<10x20xf32> {test.src_ptr} + %dest = fir.alloca !fir.array<10x20xf32> {test.dest_ptr} + %var = fir.alloca f32 + %0:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated copy from source: %{{.*}} = fir.alloca !fir.array<10x20xf32> {test.src_ptr} to destination: %{{.*}} = fir.alloca !fir.array<10x20xf32> {test.dest_ptr} + // CHECK: Generated: hlfir.assign %{{.*}} to %{{.*}} : !fir.ref>, !fir.ref> + return +} + +// ----- + +func.func @test_copy_derived_type() { + %src = fir.alloca !fir.type<_QTt{i:i32}> {test.src_ptr} + %dest = fir.alloca !fir.type<_QTt{i:i32}> {test.dest_ptr} + %var = fir.alloca f32 + %0:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated copy from source: %{{.*}} = fir.alloca !fir.type<_QTt{i:i32}> {test.src_ptr} to destination: %{{.*}} = fir.alloca !fir.type<_QTt{i:i32}> {test.dest_ptr} + // CHECK: Generated: hlfir.assign %{{.*}} to %{{.*}} : !fir.ref>, !fir.ref> + return +} + +// ----- + +func.func @test_copy_heap_scalar() { + %src = fir.allocmem f32 {test.src_ptr} + %dest = fir.allocmem f32 {test.dest_ptr} + %var = fir.alloca f32 + %0:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated copy from source: %{{.*}} = fir.allocmem f32 {test.src_ptr} to destination: %{{.*}} = fir.allocmem f32 {test.dest_ptr} + // CHECK: Generated: %{{.*}} = fir.load %{{.*}} : !fir.heap + // CHECK: Generated: fir.store %{{.*}} to %{{.*}} : !fir.heap + return +} + +// ----- + +func.func @test_copy_static_char() { + %src = fir.alloca !fir.char<1,10> {test.src_ptr} + %dest = fir.alloca !fir.char<1,10> {test.dest_ptr} + %var = fir.alloca f32 + %0:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated copy from source: %{{.*}} = fir.alloca !fir.char<1,10> {test.src_ptr} to destination: %{{.*}} = fir.alloca !fir.char<1,10> {test.dest_ptr} + // CHECK: Generated: hlfir.assign %{{.*}} to %{{.*}} : !fir.ref>, !fir.ref> + return +} + +// ----- + +func.func @test_copy_mismatched_types_fails() { + %src = fir.alloca f32 {test.src_ptr} + %dest = fir.alloca f64 {test.dest_ptr} + %var = fir.alloca f32 + %0:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Failed to generate copy from source: %{{.*}} = fir.alloca f32 {test.src_ptr} to destination: %{{.*}} = fir.alloca f64 {test.dest_ptr} + return +} + +// ----- + +func.func @test_copy_mismatched_shapes_fails() { + %src = fir.alloca !fir.array<10xf32> {test.src_ptr} + %dest = fir.alloca !fir.array<20xf32> {test.dest_ptr} + %var = fir.alloca f32 + %0:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Failed to generate copy from source: %{{.*}} = fir.alloca !fir.array<10xf32> {test.src_ptr} to destination: %{{.*}} = fir.alloca !fir.array<20xf32> {test.dest_ptr} + return +} + +// ----- + +func.func @test_copy_dynamic_array_fails(%arg0: !fir.ref>, %arg1: !fir.ref>) { + %src = fir.convert %arg0 {test.src_ptr} : (!fir.ref>) -> !fir.llvm_ptr> + %dest = fir.convert %arg1 {test.dest_ptr} : (!fir.ref>) -> !fir.llvm_ptr> + %var = fir.alloca f32 + %0:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Failed to generate copy from source: %{{.*}} = fir.convert %{{.*}} {test.src_ptr} : (!fir.ref>) -> !fir.llvm_ptr> to destination: %{{.*}} = fir.convert %{{.*}} {test.dest_ptr} : (!fir.ref>) -> !fir.llvm_ptr> + return +} + +// ----- + +func.func @test_copy_unlimited_polymorphic_fails() { + %src = fir.alloca !fir.class {test.src_ptr} + %dest = fir.alloca !fir.class {test.dest_ptr} + %var = fir.alloca f32 + %0:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Failed to generate copy from source: %{{.*}} = fir.alloca !fir.class {test.src_ptr} to destination: %{{.*}} = fir.alloca !fir.class {test.dest_ptr} + return +} + +// ----- + +func.func @test_copy_dynamic_char_fails(%arg0: !fir.ref>, %arg1: !fir.ref>) { + %src = fir.convert %arg0 {test.src_ptr} : (!fir.ref>) -> !fir.llvm_ptr> + %dest = fir.convert %arg1 {test.dest_ptr} : (!fir.ref>) -> !fir.llvm_ptr> + %var = fir.alloca f32 + %0:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Failed to generate copy from source: %{{.*}} = fir.convert %{{.*}} {test.src_ptr} : (!fir.ref>) -> !fir.llvm_ptr> to destination: %{{.*}} = fir.convert %{{.*}} {test.dest_ptr} : (!fir.ref>) -> !fir.llvm_ptr> + return +} diff --git a/flang/test/Fir/OpenACC/pointer-like-interface-free.mlir b/flang/test/Fir/OpenACC/pointer-like-interface-free.mlir new file mode 100644 index 0000000000000..6334752d9e998 --- /dev/null +++ b/flang/test/Fir/OpenACC/pointer-like-interface-free.mlir @@ -0,0 +1,94 @@ +// RUN: fir-opt %s --split-input-file --pass-pipeline="builtin.module(func.func(test-acc-pointer-like-interface{test-mode=free}))" 2>&1 | FileCheck %s + +// The tests here use a synthetic hlfir.declare in order to ensure that the hlfir dialect is +// loaded. This is required because the pass used is part of OpenACC test passes outside of +// flang and the APIs being test may generate hlfir even when it does not appear. + +func.func @test_ref_scalar_free() { + %0 = fir.alloca f32 {test.ptr} + %1:2 = hlfir.declare %0 {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated free for operation: %{{.*}} = fir.alloca f32 {test.ptr} + // CHECK-NOT: Generated + return +} + +// ----- + +func.func @test_heap_scalar_free() { + %0 = fir.allocmem f32 {test.ptr} + %var = fir.alloca f32 + %1:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated free for operation: %{{.*}} = fir.allocmem f32 {test.ptr} + // CHECK: Generated: fir.freemem %{{.*}} : !fir.heap + return +} + +// ----- + +func.func @test_heap_array_free() { + %0 = fir.allocmem !fir.array<10x20xf32> {test.ptr} + %var = fir.alloca f32 + %1:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated free for operation: %{{.*}} = fir.allocmem !fir.array<10x20xf32> {test.ptr} + // CHECK: Generated: fir.freemem %{{.*}} : !fir.heap> + return +} + +// ----- + +func.func @test_convert_walking_free() { + %0 = fir.alloca f32 + %1 = fir.convert %0 {test.ptr} : (!fir.ref) -> !fir.ptr + %2:2 = hlfir.declare %0 {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated free for operation: %{{.*}} = fir.convert %{{.*}} {test.ptr} : (!fir.ref) -> !fir.ptr + // CHECK-NOT: Generated + return +} + +// ----- + +func.func @test_declare_walking_free() { + %0 = fir.alloca f32 + %1 = fir.declare %0 {test.ptr, uniq_name = "x"} : (!fir.ref) -> !fir.ref + %2:2 = hlfir.declare %0 {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated free for operation: %{{.*}} = fir.declare %{{.*}} {test.ptr, uniq_name = "x"} : (!fir.ref) -> !fir.ref + // CHECK-NOT: Generated + return +} + +// ----- + +func.func @test_hlfir_declare_walking_free() { + %0 = fir.alloca f32 + %1:2 = hlfir.declare %0 {test.ptr, uniq_name = "x"} : (!fir.ref) -> (!fir.ref, !fir.ref) + %var = fir.alloca f32 + %2:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated free for operation + // CHECK-NOT: Generated + return +} + +// ----- + +func.func @test_heap_through_convert_free() { + %0 = fir.allocmem f32 + %1 = fir.convert %0 {test.ptr} : (!fir.heap) -> !fir.llvm_ptr + %var = fir.alloca f32 + %2:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated free for operation: %{{.*}} = fir.convert %{{.*}} {test.ptr} : (!fir.heap) -> !fir.llvm_ptr + // CHECK: Generated: %{{.*}} = fir.convert %{{.*}} : (!fir.llvm_ptr) -> !fir.heap + // CHECK: Generated: fir.freemem %{{.*}} : !fir.heap + return +} + +// ----- + +func.func @test_heap_through_declare_free() { + %0 = fir.allocmem f32 + %1 = fir.declare %0 {test.ptr, uniq_name = "x"} : (!fir.heap) -> !fir.heap + %var = fir.alloca f32 + %2:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref) -> (!fir.ref, !fir.ref) + // CHECK: Successfully generated free for operation: %{{.*}} = fir.declare %{{.*}} {test.ptr, uniq_name = "x"} : (!fir.heap) -> !fir.heap + // CHECK: Generated: fir.freemem %{{.*}} : !fir.heap + return +} diff --git a/flang/tools/fir-opt/CMakeLists.txt b/flang/tools/fir-opt/CMakeLists.txt index 4ee9752727b87..27b4e8e864074 100644 --- a/flang/tools/fir-opt/CMakeLists.txt +++ b/flang/tools/fir-opt/CMakeLists.txt @@ -7,6 +7,7 @@ if(FLANG_INCLUDE_TESTS) set(test_libs FIRTestAnalysis FIRTestOpenACCInterfaces + MLIROpenACCTestPasses MLIRTestIR ) endif() diff --git a/flang/tools/fir-opt/fir-opt.cpp b/flang/tools/fir-opt/fir-opt.cpp index d66fc3f08bdf8..cb1180bb27fbb 100644 --- a/flang/tools/fir-opt/fir-opt.cpp +++ b/flang/tools/fir-opt/fir-opt.cpp @@ -29,7 +29,10 @@ void registerTestFIROpenACCInterfacesPass(); // Defined in mlir/test, no pulic header. namespace mlir { void registerSideEffectTestPasses(); -} +namespace test { +void registerTestOpenACC(); +} // namespace test +} // namespace mlir int main(int argc, char **argv) { fir::support::registerMLIRPassesForFortranTools(); @@ -41,6 +44,7 @@ int main(int argc, char **argv) { fir::test::registerTestFIRAliasAnalysisPass(); fir::test::registerTestFIROpenACCInterfacesPass(); mlir::registerSideEffectTestPasses(); + mlir::test::registerTestOpenACC(); #endif DialectRegistry registry; fir::support::registerDialects(registry); From 1ddf3cfdc1ba501d132f1be8bb240c2fd7b6638f Mon Sep 17 00:00:00 2001 From: Razvan Lupusoru Date: Thu, 16 Oct 2025 11:28:20 -0700 Subject: [PATCH 2/2] Include box rejection and better dynamic size checks --- .../Support/FIROpenACCTypeInterfaces.cpp | 67 ++++++++++++------- 1 file changed, 43 insertions(+), 24 deletions(-) diff --git a/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp b/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp index c543ecbbef2b0..ed9e41c743754 100644 --- a/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp +++ b/flang/lib/Optimizer/OpenACC/Support/FIROpenACCTypeInterfaces.cpp @@ -757,23 +757,27 @@ mlir::Value OpenACCPointerLikeModel::genAllocate( llvm::StringRef varName, mlir::Type varType, mlir::Value originalVar, bool &needsFree) const { - // Get the element type from the pointer type - mlir::Type eleTy = mlir::cast(pointer).getElementType(); + // Unwrap to get the pointee type. + mlir::Type pointeeTy = fir::dyn_cast_ptrEleTy(pointer); + assert(pointeeTy && "expected pointee type to be extractable"); + + // Box types are descriptors that contain both metadata and a pointer to data. + // The `genAllocate` API is designed for simple allocations and cannot + // properly handle the dual nature of boxes. Using `generatePrivateInit` + // instead can allocate both the descriptor and its referenced data. For use + // cases that require an empty descriptor storage, potentially this could be + // implemented here. + if (fir::isa_box_type(pointeeTy)) + return {}; // Unlimited polymorphic (class(*)) cannot be handled - size unknown - if (fir::isUnlimitedPolymorphicType(eleTy)) + if (fir::isUnlimitedPolymorphicType(pointeeTy)) return {}; - // Character types with dynamic length cannot be handled without a descriptor. - if (auto charTy = mlir::dyn_cast(eleTy)) - if (charTy.hasDynamicLen()) - return {}; - - // Return null for dynamic or unknown shape arrays because the size of the + // Return null for dynamic size types because the size of the // allocation cannot be determined simply from the type. - if (auto seqTy = mlir::dyn_cast(eleTy)) - if (seqTy.hasDynamicExtents() || seqTy.hasUnknownShape()) - return {}; + if (fir::hasDynamicSize(pointeeTy)) + return {}; // Use heap allocation for fir.heap, stack allocation for others (fir.ref, // fir.ptr, fir.llvm_ptr). For fir.ptr, which is supposed to represent a @@ -783,10 +787,10 @@ mlir::Value OpenACCPointerLikeModel::genAllocate( mlir::Value allocation; if (std::is_same_v) { needsFree = true; - allocation = fir::AllocMemOp::create(builder, loc, eleTy); + allocation = fir::AllocMemOp::create(builder, loc, pointeeTy); } else { needsFree = false; - allocation = fir::AllocaOp::create(builder, loc, eleTy); + allocation = fir::AllocaOp::create(builder, loc, pointeeTy); } // Convert to the requested pointer type if needed. @@ -862,6 +866,17 @@ bool OpenACCPointerLikeModel::genFree( mlir::TypedValue varToFree, mlir::Value allocRes, mlir::Type varType) const { + // Unwrap to get the pointee type. + mlir::Type pointeeTy = fir::dyn_cast_ptrEleTy(pointer); + assert(pointeeTy && "expected pointee type to be extractable"); + + // Box types contain both a descriptor and data. The `genFree` API + // handles simple deallocations and cannot properly manage both parts. + // Using `generatePrivateDestroy` instead can free both the descriptor and + // its referenced data. + if (fir::isa_box_type(pointeeTy)) + return false; + // If pointer type is HeapType, assume it's a heap allocation if (std::is_same_v) { fir::FreeMemOp::create(builder, loc, varToFree); @@ -925,22 +940,26 @@ bool OpenACCPointerLikeModel::genCopy( if (source.getType() != destination.getType()) return false; - mlir::Type eleTy = mlir::cast(pointer).getElementType(); + // Unwrap to get the pointee type. + mlir::Type pointeeTy = fir::dyn_cast_ptrEleTy(pointer); + assert(pointeeTy && "expected pointee type to be extractable"); + + // Box types contain both a descriptor and referenced data. The genCopy API + // handles simple copies and cannot properly manage both parts. + if (fir::isa_box_type(pointeeTy)) + return false; // Unlimited polymorphic (class(*)) cannot be handled because source and // destination types are not known. - if (fir::isUnlimitedPolymorphicType(eleTy)) + if (fir::isUnlimitedPolymorphicType(pointeeTy)) return false; - // Dynamic lengths without descriptor do not have size information. - if (auto charTy = mlir::dyn_cast(eleTy)) - if (charTy.hasDynamicLen()) - return false; - if (auto seqTy = mlir::dyn_cast(eleTy)) - if (seqTy.hasDynamicExtents() || seqTy.hasUnknownShape()) - return false; + // Return false for dynamic size types because the copy logic + // cannot be determined simply from the type. + if (fir::hasDynamicSize(pointeeTy)) + return false; - if (fir::isa_trivial(eleTy)) { + if (fir::isa_trivial(pointeeTy)) { auto loadVal = fir::LoadOp::create(builder, loc, source); fir::StoreOp::create(builder, loc, loadVal, destination); } else {