-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[flang][acc] Implement PointerLikeType API gen[Allocate/Free/Copy] #163660
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[flang][acc] Implement PointerLikeType API gen[Allocate/Free/Copy] #163660
Conversation
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.
@llvm/pr-subscribers-openacc @llvm/pr-subscribers-flang-fir-hlfir Author: Razvan Lupusoru (razvanlupusoru) ChangesImplements genAllocate, genFree, and genCopy for FIR pointer types (fir.ref, fir.ptr, fir.heap, fir.llvm_ptr) in the OpenACC PointerLikeType interface.
Adds comprehensive MLIR tests covering various FIR types and edge cases. Patch is 27.81 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/163660.diff 7 Files Affected:
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<mlir::acc::PointerLikeType> 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<mlir::acc::PointerLikeType> varToFree,
+ mlir::Value allocRes, mlir::Type varType) const;
+
+ bool genCopy(mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc,
+ mlir::TypedValue<mlir::acc::PointerLikeType> destination,
+ mlir::TypedValue<mlir::acc::PointerLikeType> source,
+ mlir::Type varType) const;
};
template <typename T>
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<fir::PointerType>::generatePrivateDestroy(
mlir::Type type, mlir::OpBuilder &builder, mlir::Location loc,
mlir::Value privatized) const;
+template <typename Ty>
+mlir::Value OpenACCPointerLikeModel<Ty>::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<Ty>(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<fir::CharacterType>(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<fir::SequenceType>(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<Ty, fir::HeapType>) {
+ 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<Ty, fir::HeapType>) &&
+ "fir.heap is already correct type because of allocmem");
+ return fir::ConvertOp::create(builder, loc, pointer, allocation);
+ }
+
+ return allocation;
+}
+
+template mlir::Value OpenACCPointerLikeModel<fir::ReferenceType>::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<fir::PointerType>::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<fir::HeapType>::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<fir::LLVMPointerType>::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<fir::ConvertOp>(definingOp)) {
+ currentValue = convertOp.getValue();
+ continue;
+ }
+
+ if (auto viewLike = mlir::dyn_cast<mlir::ViewLikeOpInterface>(definingOp)) {
+ currentValue = viewLike.getViewSource();
+ continue;
+ }
+
+ if (stripDeclare) {
+ if (auto declareOp = mlir::dyn_cast<hlfir::DeclareOp>(definingOp)) {
+ currentValue = declareOp.getMemref();
+ continue;
+ }
+
+ if (auto declareOp = mlir::dyn_cast<fir::DeclareOp>(definingOp)) {
+ currentValue = declareOp.getMemref();
+ continue;
+ }
+ }
+ break;
+ }
+
+ return currentValue;
+}
+
+template <typename Ty>
+bool OpenACCPointerLikeModel<Ty>::genFree(
+ mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc,
+ mlir::TypedValue<mlir::acc::PointerLikeType> varToFree,
+ mlir::Value allocRes, mlir::Type varType) const {
+
+ // If pointer type is HeapType, assume it's a heap allocation
+ if (std::is_same_v<Ty, fir::HeapType>) {
+ 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<fir::AllocMemOp>(originalAlloc)) {
+ mlir::Value toFree = varToFree;
+ if (!mlir::isa<fir::HeapType>(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<fir::AllocaOp>(originalAlloc))
+ return true;
+
+ // Unable to determine allocation type
+ return false;
+}
+
+template bool OpenACCPointerLikeModel<fir::ReferenceType>::genFree(
+ mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc,
+ mlir::TypedValue<mlir::acc::PointerLikeType> varToFree,
+ mlir::Value allocRes, mlir::Type varType) const;
+
+template bool OpenACCPointerLikeModel<fir::PointerType>::genFree(
+ mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc,
+ mlir::TypedValue<mlir::acc::PointerLikeType> varToFree,
+ mlir::Value allocRes, mlir::Type varType) const;
+
+template bool OpenACCPointerLikeModel<fir::HeapType>::genFree(
+ mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc,
+ mlir::TypedValue<mlir::acc::PointerLikeType> varToFree,
+ mlir::Value allocRes, mlir::Type varType) const;
+
+template bool OpenACCPointerLikeModel<fir::LLVMPointerType>::genFree(
+ mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc,
+ mlir::TypedValue<mlir::acc::PointerLikeType> varToFree,
+ mlir::Value allocRes, mlir::Type varType) const;
+
+template <typename Ty>
+bool OpenACCPointerLikeModel<Ty>::genCopy(
+ mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc,
+ mlir::TypedValue<mlir::acc::PointerLikeType> destination,
+ mlir::TypedValue<mlir::acc::PointerLikeType> source,
+ mlir::Type varType) const {
+
+ // Check that source and destination types match
+ if (source.getType() != destination.getType())
+ return false;
+
+ mlir::Type eleTy = mlir::cast<Ty>(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<fir::CharacterType>(eleTy))
+ if (charTy.hasDynamicLen())
+ return false;
+ if (auto seqTy = mlir::dyn_cast<fir::SequenceType>(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<fir::ReferenceType>::genCopy(
+ mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc,
+ mlir::TypedValue<mlir::acc::PointerLikeType> destination,
+ mlir::TypedValue<mlir::acc::PointerLikeType> source,
+ mlir::Type varType) const;
+
+template bool OpenACCPointerLikeModel<fir::PointerType>::genCopy(
+ mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc,
+ mlir::TypedValue<mlir::acc::PointerLikeType> destination,
+ mlir::TypedValue<mlir::acc::PointerLikeType> source,
+ mlir::Type varType) const;
+
+template bool OpenACCPointerLikeModel<fir::HeapType>::genCopy(
+ mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc,
+ mlir::TypedValue<mlir::acc::PointerLikeType> destination,
+ mlir::TypedValue<mlir::acc::PointerLikeType> source,
+ mlir::Type varType) const;
+
+template bool OpenACCPointerLikeModel<fir::LLVMPointerType>::genCopy(
+ mlir::Type pointer, mlir::OpBuilder &builder, mlir::Location loc,
+ mlir::TypedValue<mlir::acc::PointerLikeType> destination,
+ mlir::TypedValue<mlir::acc::PointerLikeType> 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<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // 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<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // 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<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // 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<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // 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<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // 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<f32>) -> !fir.ptr<f32>
+ %2:2 = hlfir.declare %0 {uniq_name = "load_hlfir"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // CHECK: Successfully generated alloc for operation
+ // CHECK: Generated: %{{.*}} = fir.alloca f32
+ // CHECK: Generated: %{{.*}} = fir.convert %{{.*}} : (!fir.ref<f32>) -> !fir.ptr<f32>
+ return
+}
+
+// -----
+
+func.func @test_llvm_ptr_scalar_alloc() {
+ %0 = fir.alloca f32
+ %1 = fir.convert %0 {test.ptr} : (!fir.ref<f32>) -> !fir.llvm_ptr<f32>
+ %2:2 = hlfir.declare %0 {uniq_name = "load_hlfir"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // CHECK: Successfully generated alloc for operation
+ // CHECK: Generated: %{{.*}} = fir.alloca f32
+ // CHECK: Generated: %{{.*}} = fir.convert %{{.*}} : (!fir.ref<f32>) -> !fir.llvm_ptr<f32>
+ return
+}
+
+// -----
+
+func.func @test_dynamic_array_alloc_fails(%arg0: !fir.ref<!fir.array<?xf32>>) {
+ %0 = fir.convert %arg0 {test.ptr} : (!fir.ref<!fir.array<?xf32>>) -> !fir.llvm_ptr<!fir.array<?xf32>>
+ %var = fir.alloca f32
+ %1:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // CHECK: Failed to generate alloc for operation: %{{.*}} = fir.convert %{{.*}} {test.ptr} : (!fir.ref<!fir.array<?xf32>>) -> !fir.llvm_ptr<!fir.array<?xf32>>
+ return
+}
+
+// -----
+
+func.func @test_unlimited_polymorphic_alloc_fails() {
+ %0 = fir.alloca !fir.class<none> {test.ptr}
+ %var = fir.alloca f32
+ %1:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // CHECK: Failed to generate alloc for operation: %{{.*}} = fir.alloca !fir.class<none> {test.ptr}
+ return
+}
+
+// -----
+
+func.func @test_dynamic_char_alloc_fails(%arg0: !fir.ref<!fir.char<1,?>>) {
+ %0 = fir.convert %arg0 {test.ptr} : (!fir.ref<!fir.char<1,?>>) -> !fir.llvm_ptr<!fir.char<1,?>>
+ %var = fir.alloca f32
+ %1:2 = hlfir.declare %var {uniq_name = "load_hlfir"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // CHECK: Failed to generate alloc for operation: %{{.*}} = fir.convert %{{.*}} {test.ptr} : (!fir.ref<!fir.char<1,?>>) -> !fir.llvm_ptr<!fir.char<1,?>>
+ 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<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // 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<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // 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<f32>
+ // CHECK: Generated: fir.store %{{.*}} to %{{.*}} : !fir.ref<f32>
+ 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<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // 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.array<10x20xf32>>, !fir.ref<!fir.array<10x20xf32>>
+ 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<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // 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.type<_QTt{i:i32}>>, !fir.ref<!fir.type<_QTt{i:i32}>>
+ 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<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // 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<f32>
+ // CHECK: Generated: fir.store %{{.*}} to %{{.*}} : !fir.heap<f32>
+ 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<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // 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.char<1,10>>, !fir.ref<!fir.char<1,10>>
+ 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<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+ // 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() {
+ ...
[truncated]
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Razvan, I added some questions/suggestions.
// 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<fir::CharacterType>(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<fir::SequenceType>(eleTy)) | ||
if (seqTy.hasDynamicExtents() || seqTy.hasUnknownShape()) | ||
return {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fir::hasDynamicSize(eleTy)
should be equivalent to what you have here with the exception of how fir.ref<fir.box<None>>
/fir.ref<fir.class<None>>
is handled.
Which leads me to another question, what is the expected behavior of fir.ref<fir.box/class<T>>
? Is it to allocate/free/copy descriptors or to also allocate/free/copy the target data of the descriptor?
fir::hasDynamicSize
will accept fir.ref<fir.box>
. If you want to reject them, you can use llvm::isa<fir::BaseBoxType>(eleTy)
to detect them.
Currently, I think your code is not rejecting !fir.ref<fir.box<!fir.array<?xf32>>>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fir::hasDynamicSize(eleTy) should be equivalent
Thank you for the suggestion to use hasDynamicSize - I will use that instead.
Which leads me to another question, what is the expected behavior of fir.ref<fir.box/class>? Is it to allocate/free/copy descriptors or to also allocate/free/copy the target data of the descriptor?
I think that genAllocate
should allow for simple shallow allocations - so allocated descriptor storage is enough. generatePrivateInit
from MappableType (which is accessible through both fir.ref<fir.box>
and fir.box
should handle the complexity of allocating both descriptor and data.
However, I have decided in the current PR, I will reject boxes in this API and we can revisit if we need shallow allocations of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
// 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<fir::CharacterType>(eleTy)) | ||
if (charTy.hasDynamicLen()) | ||
return false; | ||
if (auto seqTy = mlir::dyn_cast<fir::SequenceType>(eleTy)) | ||
if (seqTy.hasDynamicExtents() || seqTy.hasUnknownShape()) | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, assuming you do not want to deal with with fir::ref<fir.box>, this could be simplified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix. Thank you for catching this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
auto loadVal = fir::LoadOp::create(builder, loc, source); | ||
fir::StoreOp::create(builder, loc, loadVal, destination); | ||
} else { | ||
hlfir::AssignOp::create(builder, loc, source, destination); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that if this interface is meant to handle the fir.ref case, this will copy the data, not the descriptors, which is not in line with the current impl of genAllocate/Free that allocate/free the descriptors.
Side note that there is also a fir.copy type that can lower to memcopy/memmove. It will however not perform any deep copies of allocatable components, nor call the user defined assignments on components that are derived type (I am still not sure we want to call those).
If doing deep copies/calling user defined assignments on components is what you intend, calling hlfir.assign is the right solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch. I decided to reject fir.ref<fir.box> for now in all of the APIs to prevent this misuse/misunderstanding regarding descriptor and its data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/17/builds/11965 Here is the relevant piece of the build log for the reference
|
The reported buildbot failure passed on rerun: https://lab.llvm.org/buildbot/#/builders/17/builds/11968/steps/7/logs/stdio |
Implements genAllocate, genFree, and genCopy for FIR pointer types (fir.ref, fir.ptr, fir.heap, fir.llvm_ptr) in the OpenACC PointerLikeType interface.
Adds comprehensive MLIR tests covering various FIR types and edge cases.