[flang-commits] [flang] [flang][openacc] Place post allocate/deallocate attribute correctly (PR #79883)

Valentin Clement バレンタイン クレメン via flang-commits flang-commits at lists.llvm.org
Mon Jan 29 14:46:14 PST 2024


https://github.com/clementval updated https://github.com/llvm/llvm-project/pull/79883

>From 7f8778c131fa508628588ae2f37873bb574de989 Mon Sep 17 00:00:00 2001
From: Valentin Clement <clementval at gmail.com>
Date: Mon, 29 Jan 2024 11:01:48 -0800
Subject: [PATCH 1/2] [flang][openacc] Place post allocate/deallocate attribute
 correctly

The `acc.declate_action` attribute was sometime misplaced as reported
in #79770.
This patch updates the lowering code to place the postAllocate/postDeallocate
actions at the correct place.
---
 flang/lib/Lower/Allocatable.cpp          | 60 ++++++++++++++----------
 flang/test/Lower/OpenACC/acc-declare.f90 | 12 +++++
 2 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/flang/lib/Lower/Allocatable.cpp b/flang/lib/Lower/Allocatable.cpp
index affb483e5b82b43..71b61b9c20f63a3 100644
--- a/flang/lib/Lower/Allocatable.cpp
+++ b/flang/lib/Lower/Allocatable.cpp
@@ -388,7 +388,6 @@ class AllocateStmtHelper {
       genSourceMoldAllocation(alloc, boxAddr, /*isSource=*/false);
     else
       genSimpleAllocation(alloc, boxAddr);
-    postAllocationAction(alloc);
   }
 
   static bool lowerBoundsAreOnes(const Allocation &alloc) {
@@ -458,6 +457,7 @@ class AllocateStmtHelper {
       // Pointers must use PointerAllocate so that their deallocations
       // can be validated.
       genInlinedAllocation(alloc, box);
+      postAllocationAction(alloc);
       return;
     }
     // Generate a sequence of runtime calls.
@@ -471,6 +471,7 @@ class AllocateStmtHelper {
     genAllocateObjectBounds(alloc, box);
     mlir::Value stat = genRuntimeAllocate(builder, loc, box, errorManager);
     fir::factory::syncMutableBoxFromIRBox(builder, loc, box);
+    postAllocationAction(alloc);
     errorManager.assignStat(builder, loc, stat);
   }
 
@@ -601,6 +602,7 @@ class AllocateStmtHelper {
     else
       stat = genRuntimeAllocate(builder, loc, box, errorManager);
     fir::factory::syncMutableBoxFromIRBox(builder, loc, box);
+    postAllocationAction(alloc);
     errorManager.assignStat(builder, loc, stat);
   }
 
@@ -739,16 +741,37 @@ void Fortran::lower::genAllocateStmt(
 // Deallocate statement implementation
 //===----------------------------------------------------------------------===//
 
+static void preDeallocationAction(Fortran::lower::AbstractConverter &converter,
+                                  fir::FirOpBuilder &builder,
+                                  mlir::Value beginOpValue,
+                                  const Fortran::semantics::Symbol &sym) {
+  if (sym.test(Fortran::semantics::Symbol::Flag::AccDeclare))
+    Fortran::lower::attachDeclarePreDeallocAction(converter, builder,
+                                                  beginOpValue, sym);
+}
+
+static void postDeallocationAction(Fortran::lower::AbstractConverter &converter,
+                                   fir::FirOpBuilder &builder,
+                                   const Fortran::semantics::Symbol &sym) {
+  if (sym.test(Fortran::semantics::Symbol::Flag::AccDeclare))
+    Fortran::lower::attachDeclarePostDeallocAction(converter, builder, sym);
+}
+
 // Generate deallocation of a pointer/allocatable.
-static mlir::Value genDeallocate(fir::FirOpBuilder &builder, mlir::Location loc,
-                                 const fir::MutableBoxValue &box,
-                                 ErrorManager &errorManager,
-                                 mlir::Value declaredTypeDesc = {}) {
+static mlir::Value
+genDeallocate(fir::FirOpBuilder &builder,
+              Fortran::lower::AbstractConverter &converter, mlir::Location loc,
+              const fir::MutableBoxValue &box, ErrorManager &errorManager,
+              mlir::Value declaredTypeDesc = {},
+              const Fortran::semantics::Symbol *symbol = nullptr) {
   // Deallocate intrinsic types inline.
   if (!box.isDerived() && !box.isPolymorphic() &&
       !box.isUnlimitedPolymorphic() && !errorManager.hasStatSpec() &&
       !useAllocateRuntime) {
-    return fir::factory::genFreemem(builder, loc, box);
+    mlir::Value ret = fir::factory::genFreemem(builder, loc, box);
+    if (symbol)
+      postDeallocationAction(converter, builder, *symbol);
+    return ret;
   }
   // Use runtime calls to deallocate descriptor cases. Sync MutableBoxValue
   // with its descriptor before and after calls if needed.
@@ -756,6 +779,8 @@ static mlir::Value genDeallocate(fir::FirOpBuilder &builder, mlir::Location loc,
   mlir::Value stat =
       genRuntimeDeallocate(builder, loc, box, errorManager, declaredTypeDesc);
   fir::factory::syncMutableBoxFromIRBox(builder, loc, box);
+  if (symbol)
+    postDeallocationAction(converter, builder, *symbol);
   errorManager.assignStat(builder, loc, stat);
   return stat;
 }
@@ -769,7 +794,7 @@ void Fortran::lower::genDeallocateBox(
   ErrorManager errorManager;
   errorManager.init(converter, loc, statExpr, errMsgExpr);
   fir::FirOpBuilder &builder = converter.getFirOpBuilder();
-  genDeallocate(builder, loc, box, errorManager, declaredTypeDesc);
+  genDeallocate(builder, converter, loc, box, errorManager, declaredTypeDesc);
 }
 
 void Fortran::lower::genDeallocateIfAllocated(
@@ -792,22 +817,6 @@ void Fortran::lower::genDeallocateIfAllocated(
       .end();
 }
 
-static void preDeallocationAction(Fortran::lower::AbstractConverter &converter,
-                                  fir::FirOpBuilder &builder,
-                                  mlir::Value beginOpValue,
-                                  const Fortran::semantics::Symbol &sym) {
-  if (sym.test(Fortran::semantics::Symbol::Flag::AccDeclare))
-    Fortran::lower::attachDeclarePreDeallocAction(converter, builder,
-                                                  beginOpValue, sym);
-}
-
-static void postDeallocationAction(Fortran::lower::AbstractConverter &converter,
-                                   fir::FirOpBuilder &builder,
-                                   const Fortran::semantics::Symbol &sym) {
-  if (sym.test(Fortran::semantics::Symbol::Flag::AccDeclare))
-    Fortran::lower::attachDeclarePostDeallocAction(converter, builder, sym);
-}
-
 void Fortran::lower::genDeallocateStmt(
     Fortran::lower::AbstractConverter &converter,
     const Fortran::parser::DeallocateStmt &stmt, mlir::Location loc) {
@@ -843,10 +852,9 @@ void Fortran::lower::genDeallocateStmt(
               Fortran::lower::getTypeDescAddr(converter, loc, *derivedTypeSpec);
         }
     }
-    mlir::Value beginOpValue =
-        genDeallocate(builder, loc, box, errorManager, declaredTypeDesc);
+    mlir::Value beginOpValue = genDeallocate(
+        builder, converter, loc, box, errorManager, declaredTypeDesc, &symbol);
     preDeallocationAction(converter, builder, beginOpValue, symbol);
-    postDeallocationAction(converter, builder, symbol);
   }
   builder.restoreInsertionPoint(insertPt);
 }
diff --git a/flang/test/Lower/OpenACC/acc-declare.f90 b/flang/test/Lower/OpenACC/acc-declare.f90
index 157eedf3165c12e..93060fbbfa802c0 100644
--- a/flang/test/Lower/OpenACC/acc-declare.f90
+++ b/flang/test/Lower/OpenACC/acc-declare.f90
@@ -318,6 +318,18 @@ subroutine acc_declare_array_section(a)
 
 ! CHECK: acc.copyout accPtr(%[[COPYIN]] : !fir.ref<!fir.array<?xi32>>) bounds(%{{.*}}) to varPtr(%[[BOX_ADDR]] : !fir.ref<!fir.array<?xi32>>) {dataClause = #acc<data_clause acc_copy>, name = "a(1:10)"}
 
+  subroutine acc_declare_allocate_with_stat()
+    integer :: status
+    real, pointer, dimension(:) :: localptr
+    !$acc declare create(localptr)
+    allocate(localptr(n), stat=status)
+
+    deallocate(localptr, stat=status)
+  end subroutine
+
+! CHECK-LABEL: func.func @_QMacc_declarePacc_declare_allocate_with_stat()
+! CHECK: fir.call @_FortranAPointerAllocate(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {{.*}} {acc.declare_action = #acc.declare_action<postAlloc = @_QMacc_declareFacc_declare_allocate_with_statElocalptr_acc_declare_update_desc_post_alloc>}
+! CHECK: fir.call @_FortranAPointerDeallocate(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {{.*}} {acc.declare_action = #acc.declare_action<preDealloc = @_QMacc_declareFacc_declare_allocate_with_statElocalptr_acc_declare_update_desc_pre_dealloc>}
 end module
 
 module acc_declare_allocatable_test

>From 1e0dec54f1920d5bff045df1a007644ea01365de Mon Sep 17 00:00:00 2001
From: Valentin Clement <clementval at gmail.com>
Date: Mon, 29 Jan 2024 14:45:40 -0800
Subject: [PATCH 2/2] Fix attribute override

---
 flang/lib/Lower/OpenACC.cpp              | 69 +++++++++++++++++-------
 flang/test/Lower/OpenACC/acc-declare.f90 |  2 +-
 2 files changed, 52 insertions(+), 19 deletions(-)

diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index d3e8dd28ffd6394..43f54c6d2a71bb4 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -4094,12 +4094,23 @@ void Fortran::lower::attachDeclarePostAllocAction(
   std::stringstream fctName;
   fctName << converter.mangleName(sym) << declarePostAllocSuffix.str();
   mlir::Operation &op = builder.getInsertionBlock()->back();
-  op.setAttr(mlir::acc::getDeclareActionAttrName(),
-             mlir::acc::DeclareActionAttr::get(
-                 builder.getContext(),
-                 /*preAlloc=*/{},
-                 /*postAlloc=*/builder.getSymbolRefAttr(fctName.str()),
-                 /*preDealloc=*/{}, /*postDealloc=*/{}));
+
+  if (op.hasAttr(mlir::acc::getDeclareActionAttrName())) {
+    auto attr = op.getAttrOfType<mlir::acc::DeclareActionAttr>(
+        mlir::acc::getDeclareActionAttrName());
+    op.setAttr(mlir::acc::getDeclareActionAttrName(),
+               mlir::acc::DeclareActionAttr::get(
+                   builder.getContext(), attr.getPreAlloc(),
+                   /*postAlloc=*/builder.getSymbolRefAttr(fctName.str()),
+                   attr.getPreDealloc(), attr.getPostDealloc()));
+  } else {
+    op.setAttr(mlir::acc::getDeclareActionAttrName(),
+               mlir::acc::DeclareActionAttr::get(
+                   builder.getContext(),
+                   /*preAlloc=*/{},
+                   /*postAlloc=*/builder.getSymbolRefAttr(fctName.str()),
+                   /*preDealloc=*/{}, /*postDealloc=*/{}));
+  }
 }
 
 void Fortran::lower::attachDeclarePreDeallocAction(
@@ -4115,13 +4126,25 @@ void Fortran::lower::attachDeclarePreDeallocAction(
 
   std::stringstream fctName;
   fctName << converter.mangleName(sym) << declarePreDeallocSuffix.str();
-  beginOpValue.getDefiningOp()->setAttr(
-      mlir::acc::getDeclareActionAttrName(),
-      mlir::acc::DeclareActionAttr::get(
-          builder.getContext(),
-          /*preAlloc=*/{}, /*postAlloc=*/{},
-          /*preDealloc=*/builder.getSymbolRefAttr(fctName.str()),
-          /*postDealloc=*/{}));
+
+  auto *op = beginOpValue.getDefiningOp();
+  if (op->hasAttr(mlir::acc::getDeclareActionAttrName())) {
+    auto attr = op->getAttrOfType<mlir::acc::DeclareActionAttr>(
+        mlir::acc::getDeclareActionAttrName());
+    op->setAttr(mlir::acc::getDeclareActionAttrName(),
+                mlir::acc::DeclareActionAttr::get(
+                    builder.getContext(), attr.getPreAlloc(),
+                    attr.getPostAlloc(),
+                    /*preDealloc=*/builder.getSymbolRefAttr(fctName.str()),
+                    attr.getPostDealloc()));
+  } else {
+    op->setAttr(mlir::acc::getDeclareActionAttrName(),
+                mlir::acc::DeclareActionAttr::get(
+                    builder.getContext(),
+                    /*preAlloc=*/{}, /*postAlloc=*/{},
+                    /*preDealloc=*/builder.getSymbolRefAttr(fctName.str()),
+                    /*postDealloc=*/{}));
+  }
 }
 
 void Fortran::lower::attachDeclarePostDeallocAction(
@@ -4138,11 +4161,21 @@ void Fortran::lower::attachDeclarePostDeallocAction(
   std::stringstream fctName;
   fctName << converter.mangleName(sym) << declarePostDeallocSuffix.str();
   mlir::Operation &op = builder.getInsertionBlock()->back();
-  op.setAttr(mlir::acc::getDeclareActionAttrName(),
-             mlir::acc::DeclareActionAttr::get(
-                 builder.getContext(),
-                 /*preAlloc=*/{}, /*postAlloc=*/{}, /*preDealloc=*/{},
-                 /*postDealloc=*/builder.getSymbolRefAttr(fctName.str())));
+  if (op.hasAttr(mlir::acc::getDeclareActionAttrName())) {
+    auto attr = op.getAttrOfType<mlir::acc::DeclareActionAttr>(
+        mlir::acc::getDeclareActionAttrName());
+    op.setAttr(mlir::acc::getDeclareActionAttrName(),
+               mlir::acc::DeclareActionAttr::get(
+                   builder.getContext(), attr.getPreAlloc(),
+                   attr.getPostAlloc(), attr.getPreDealloc(),
+                   /*postDealloc=*/builder.getSymbolRefAttr(fctName.str())));
+  } else {
+    op.setAttr(mlir::acc::getDeclareActionAttrName(),
+               mlir::acc::DeclareActionAttr::get(
+                   builder.getContext(),
+                   /*preAlloc=*/{}, /*postAlloc=*/{}, /*preDealloc=*/{},
+                   /*postDealloc=*/builder.getSymbolRefAttr(fctName.str())));
+  }
 }
 
 void Fortran::lower::genOpenACCTerminator(fir::FirOpBuilder &builder,
diff --git a/flang/test/Lower/OpenACC/acc-declare.f90 b/flang/test/Lower/OpenACC/acc-declare.f90
index 93060fbbfa802c0..94fb4125022de3a 100644
--- a/flang/test/Lower/OpenACC/acc-declare.f90
+++ b/flang/test/Lower/OpenACC/acc-declare.f90
@@ -329,7 +329,7 @@ subroutine acc_declare_allocate_with_stat()
 
 ! CHECK-LABEL: func.func @_QMacc_declarePacc_declare_allocate_with_stat()
 ! CHECK: fir.call @_FortranAPointerAllocate(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {{.*}} {acc.declare_action = #acc.declare_action<postAlloc = @_QMacc_declareFacc_declare_allocate_with_statElocalptr_acc_declare_update_desc_post_alloc>}
-! CHECK: fir.call @_FortranAPointerDeallocate(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {{.*}} {acc.declare_action = #acc.declare_action<preDealloc = @_QMacc_declareFacc_declare_allocate_with_statElocalptr_acc_declare_update_desc_pre_dealloc>}
+! CHECK: fir.call @_FortranAPointerDeallocate(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {{.*}} {acc.declare_action = #acc.declare_action<preDealloc = @_QMacc_declareFacc_declare_allocate_with_statElocalptr_acc_declare_update_desc_pre_dealloc, postDealloc = @_QMacc_declareFacc_declare_allocate_with_statElocalptr_acc_declare_update_desc_post_dealloc>}
 end module
 
 module acc_declare_allocatable_test



More information about the flang-commits mailing list