[flang][acc] Ensure data exit action is generated for present & nocreate #126560

razvanlupusoru · 2025-02-10T17:59:49Z

The acc.delete operation has semantics of decrementing present counter and deleting the data when the counter reaches zero. Since both acc.present and acc.nocreate are both intended to increment present counter, this matching exit action must be inserted.

This is also what was specified in OpenACC dialect documentation: https://mlir.llvm.org/docs/Dialects/OpenACCDialect/#operation-categories

The acc.delete operation has semantics of decrementing present counter and deleting the data when the counter reaches zero. Since both acc.present and acc.nocreate are both intended to increment present counter, this matching exit action must be inserted. This is also what was specified in OpenACC dialect documentation: https://mlir.llvm.org/docs/Dialects/OpenACCDialect/#operation-categories

llvmbot · 2025-02-10T18:00:23Z

@llvm/pr-subscribers-openacc

@llvm/pr-subscribers-flang-fir-hlfir

Author: Razvan Lupusoru (razvanlupusoru)

Changes

The acc.delete operation has semantics of decrementing present counter and deleting the data when the counter reaches zero. Since both acc.present and acc.nocreate are both intended to increment present counter, this matching exit action must be inserted.

This is also what was specified in OpenACC dialect documentation: https://mlir.llvm.org/docs/Dialects/OpenACCDialect/#operation-categories

Full diff: https://github.com/llvm/llvm-project/pull/126560.diff

8 Files Affected:

(modified) flang/lib/Lower/OpenACC.cpp (+24-3)
(modified) flang/test/Lower/OpenACC/acc-kernels-loop.f90 (+4)
(modified) flang/test/Lower/OpenACC/acc-kernels.f90 (+6)
(modified) flang/test/Lower/OpenACC/acc-parallel-loop.f90 (+4)
(modified) flang/test/Lower/OpenACC/acc-parallel.f90 (+5-1)
(modified) flang/test/Lower/OpenACC/acc-serial-loop.f90 (+4)
(modified) flang/test/Lower/OpenACC/acc-serial.f90 (+6)
(modified) mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp (+1)

diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index ac1a1c00eb145f8..da534107b15e409 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -2281,8 +2281,8 @@ static Op createComputeOp(
   mlir::Value selfCond;
   llvm::SmallVector<mlir::Value> waitOperands, attachEntryOperands,
       copyEntryOperands, copyinEntryOperands, copyoutEntryOperands,
-      createEntryOperands, dataClauseOperands, numGangs, numWorkers,
-      vectorLength, async;
+      createEntryOperands, nocreateEntryOperands, presentEntryOperands,
+      dataClauseOperands, numGangs, numWorkers, vectorLength, async;
   llvm::SmallVector<mlir::Attribute> numGangsDeviceTypes, numWorkersDeviceTypes,
       vectorLengthDeviceTypes, asyncDeviceTypes, asyncOnlyDeviceTypes,
       waitOperandsDeviceTypes, waitOnlyDeviceTypes;
@@ -2457,19 +2457,25 @@ static Op createComputeOp(
     } else if (const auto *noCreateClause =
                    std::get_if<Fortran::parser::AccClause::NoCreate>(
                        &clause.u)) {
+      auto crtDataStart = dataClauseOperands.size();
       genDataOperandOperations<mlir::acc::NoCreateOp>(
           noCreateClause->v, converter, semanticsContext, stmtCtx,
           dataClauseOperands, mlir::acc::DataClause::acc_no_create,
           /*structured=*/true, /*implicit=*/false, async, asyncDeviceTypes,
           asyncOnlyDeviceTypes);
+      nocreateEntryOperands.append(dataClauseOperands.begin() + crtDataStart,
+          dataClauseOperands.end());
     } else if (const auto *presentClause =
                    std::get_if<Fortran::parser::AccClause::Present>(
                        &clause.u)) {
+      auto crtDataStart = dataClauseOperands.size();
       genDataOperandOperations<mlir::acc::PresentOp>(
           presentClause->v, converter, semanticsContext, stmtCtx,
           dataClauseOperands, mlir::acc::DataClause::acc_present,
           /*structured=*/true, /*implicit=*/false, async, asyncDeviceTypes,
           asyncOnlyDeviceTypes);
+      presentEntryOperands.append(dataClauseOperands.begin() + crtDataStart,
+          dataClauseOperands.end());
     } else if (const auto *devicePtrClause =
                    std::get_if<Fortran::parser::AccClause::Deviceptr>(
                        &clause.u)) {
@@ -2634,6 +2640,10 @@ static Op createComputeOp(
       builder, attachEntryOperands, /*structured=*/true);
   genDataExitOperations<mlir::acc::CreateOp, mlir::acc::DeleteOp>(
       builder, createEntryOperands, /*structured=*/true);
+  genDataExitOperations<mlir::acc::NoCreateOp, mlir::acc::DeleteOp>(
+      builder, nocreateEntryOperands, /*structured=*/true);
+  genDataExitOperations<mlir::acc::PresentOp, mlir::acc::DeleteOp>(
+      builder, presentEntryOperands, /*structured=*/true);
 
   builder.restoreInsertionPoint(insPt);
   return computeOp;
@@ -2648,7 +2658,8 @@ static void genACCDataOp(Fortran::lower::AbstractConverter &converter,
   mlir::Value ifCond;
   llvm::SmallVector<mlir::Value> attachEntryOperands, createEntryOperands,
       copyEntryOperands, copyinEntryOperands, copyoutEntryOperands,
-      dataClauseOperands, waitOperands, async;
+      nocreateEntryOperands, presentEntryOperands, dataClauseOperands,
+      waitOperands, async;
   llvm::SmallVector<mlir::Attribute> asyncDeviceTypes, asyncOnlyDeviceTypes,
       waitOperandsDeviceTypes, waitOnlyDeviceTypes;
   llvm::SmallVector<int32_t> waitOperandsSegments;
@@ -2745,19 +2756,25 @@ static void genACCDataOp(Fortran::lower::AbstractConverter &converter,
     } else if (const auto *noCreateClause =
                    std::get_if<Fortran::parser::AccClause::NoCreate>(
                        &clause.u)) {
+      auto crtDataStart = dataClauseOperands.size();
       genDataOperandOperations<mlir::acc::NoCreateOp>(
           noCreateClause->v, converter, semanticsContext, stmtCtx,
           dataClauseOperands, mlir::acc::DataClause::acc_no_create,
           /*structured=*/true, /*implicit=*/false, async, asyncDeviceTypes,
           asyncOnlyDeviceTypes);
+      nocreateEntryOperands.append(dataClauseOperands.begin() + crtDataStart,
+          dataClauseOperands.end());
     } else if (const auto *presentClause =
                    std::get_if<Fortran::parser::AccClause::Present>(
                        &clause.u)) {
+      auto crtDataStart = dataClauseOperands.size();
       genDataOperandOperations<mlir::acc::PresentOp>(
           presentClause->v, converter, semanticsContext, stmtCtx,
           dataClauseOperands, mlir::acc::DataClause::acc_present,
           /*structured=*/true, /*implicit=*/false, async, asyncDeviceTypes,
           asyncOnlyDeviceTypes);
+      presentEntryOperands.append(dataClauseOperands.begin() + crtDataStart,
+          dataClauseOperands.end());
     } else if (const auto *deviceptrClause =
                    std::get_if<Fortran::parser::AccClause::Deviceptr>(
                        &clause.u)) {
@@ -2837,6 +2854,10 @@ static void genACCDataOp(Fortran::lower::AbstractConverter &converter,
       builder, attachEntryOperands, /*structured=*/true);
   genDataExitOperations<mlir::acc::CreateOp, mlir::acc::DeleteOp>(
       builder, createEntryOperands, /*structured=*/true);
+  genDataExitOperations<mlir::acc::NoCreateOp, mlir::acc::DeleteOp>(
+      builder, nocreateEntryOperands, /*structured=*/true);
+  genDataExitOperations<mlir::acc::PresentOp, mlir::acc::DeleteOp>(
+      builder, presentEntryOperands, /*structured=*/true);
 
   builder.restoreInsertionPoint(insPt);
 }
diff --git a/flang/test/Lower/OpenACC/acc-kernels-loop.f90 b/flang/test/Lower/OpenACC/acc-kernels-loop.f90
index 182b512f3931ede..aa5d65afd57ec4e 100644
--- a/flang/test/Lower/OpenACC/acc-kernels-loop.f90
+++ b/flang/test/Lower/OpenACC/acc-kernels-loop.f90
@@ -393,6 +393,8 @@ subroutine acc_kernels_loop
 ! CHECK-NEXT:   }{{$}}
 ! CHECK:        acc.terminator
 ! CHECK-NEXT: }{{$}}
+! CHECK:      acc.delete accPtr(%[[NOCREATE_A]] : !fir.ref<!fir.array<10xf32>>) {dataClause = #acc<data_clause acc_no_create>, name = "a"}
+! CHECK:      acc.delete accPtr(%[[NOCREATE_B]] : !fir.ref<!fir.array<10xf32>>) {dataClause = #acc<data_clause acc_no_create>, name = "b"}
 
   !$acc kernels loop present(a, b)
   DO i = 1, n
@@ -407,6 +409,8 @@ subroutine acc_kernels_loop
 ! CHECK-NEXT:   }{{$}}
 ! CHECK:        acc.terminator
 ! CHECK-NEXT: }{{$}}
+! CHECK:      acc.delete accPtr(%[[PRESENT_A]] : !fir.ref<!fir.array<10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "a"}
+! CHECK:      acc.delete accPtr(%[[PRESENT_B]] : !fir.ref<!fir.array<10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "b"}
 
   !$acc kernels loop deviceptr(a) deviceptr(b)
   DO i = 1, n
diff --git a/flang/test/Lower/OpenACC/acc-kernels.f90 b/flang/test/Lower/OpenACC/acc-kernels.f90
index f25a9d411098b18..b333bb981a79bb2 100644
--- a/flang/test/Lower/OpenACC/acc-kernels.f90
+++ b/flang/test/Lower/OpenACC/acc-kernels.f90
@@ -253,6 +253,9 @@ subroutine acc_kernels
 ! CHECK:      acc.kernels dataOperands(%[[NO_CREATE_A]], %[[NO_CREATE_B]], %[[CREATE_C]] : !fir.ref<!fir.array<10x10xf32>>, !fir.ref<!fir.array<10x10xf32>>, !fir.ref<!fir.array<10x10xf32>>) {
 ! CHECK:        acc.terminator
 ! CHECK-NEXT: }{{$}}
+! CHECK: acc.delete accPtr(%[[CREATE_C]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_create_zero>, name = "c"}
+! CHECK: acc.delete accPtr(%[[NO_CREATE_A]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_no_create>, name = "a"}
+! CHECK: acc.delete accPtr(%[[NO_CREATE_B]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_no_create>, name = "b"}
 
   !$acc kernels present(a, b, c)
   !$acc end kernels
@@ -263,6 +266,9 @@ subroutine acc_kernels
 ! CHECK:      acc.kernels dataOperands(%[[PRESENT_A]], %[[PRESENT_B]], %[[PRESENT_C]] : !fir.ref<!fir.array<10x10xf32>>, !fir.ref<!fir.array<10x10xf32>>, !fir.ref<!fir.array<10x10xf32>>) {
 ! CHECK:        acc.terminator
 ! CHECK-NEXT: }{{$}}
+! CHECK: acc.delete accPtr(%[[PRESENT_A]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "a"}
+! CHECK: acc.delete accPtr(%[[PRESENT_B]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "b"}
+! CHECK: acc.delete accPtr(%[[PRESENT_C]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "c"}
 
   !$acc kernels deviceptr(a) deviceptr(c)
   !$acc end kernels
diff --git a/flang/test/Lower/OpenACC/acc-parallel-loop.f90 b/flang/test/Lower/OpenACC/acc-parallel-loop.f90
index 6f4409eaf56000d..7a41da320b955cc 100644
--- a/flang/test/Lower/OpenACC/acc-parallel-loop.f90
+++ b/flang/test/Lower/OpenACC/acc-parallel-loop.f90
@@ -393,6 +393,8 @@ subroutine acc_parallel_loop
 ! CHECK-NEXT:   }{{$}}
 ! CHECK:        acc.yield
 ! CHECK-NEXT: }{{$}}
+! CHECK:      acc.delete accPtr(%[[NOCREATE_A]] : !fir.ref<!fir.array<10xf32>>) {dataClause = #acc<data_clause acc_no_create>, name = "a"}
+! CHECK:      acc.delete accPtr(%[[NOCREATE_B]] : !fir.ref<!fir.array<10xf32>>) {dataClause = #acc<data_clause acc_no_create>, name = "b"}
 
   !$acc parallel loop present(a, b)
   DO i = 1, n
@@ -407,6 +409,8 @@ subroutine acc_parallel_loop
 ! CHECK-NEXT:   }{{$}}
 ! CHECK:        acc.yield
 ! CHECK-NEXT: }{{$}}
+! CHECK:      acc.delete accPtr(%[[PRESENT_A]] : !fir.ref<!fir.array<10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "a"}
+! CHECK:      acc.delete accPtr(%[[PRESENT_B]] : !fir.ref<!fir.array<10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "b"}
 
   !$acc parallel loop deviceptr(a) deviceptr(b)
   DO i = 1, n
diff --git a/flang/test/Lower/OpenACC/acc-parallel.f90 b/flang/test/Lower/OpenACC/acc-parallel.f90
index 7a51be21e914dd6..e16c1b218878c31 100644
--- a/flang/test/Lower/OpenACC/acc-parallel.f90
+++ b/flang/test/Lower/OpenACC/acc-parallel.f90
@@ -291,7 +291,8 @@ subroutine acc_parallel
 ! CHECK:        acc.yield
 ! CHECK-NEXT: }{{$}}
 ! CHECK: acc.delete accPtr(%[[CREATE_C]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_create_zero>, name = "c"}
-
+! CHECK: acc.delete accPtr(%[[NO_CREATE_A]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_no_create>, name = "a"}
+! CHECK: acc.delete accPtr(%[[NO_CREATE_B]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_no_create>, name = "b"}
 
   !$acc parallel present(a, b, c)
   !$acc end parallel
@@ -302,6 +303,9 @@ subroutine acc_parallel
 ! CHECK:      acc.parallel dataOperands(%[[PRESENT_A]], %[[PRESENT_B]], %[[PRESENT_C]] : !fir.ref<!fir.array<10x10xf32>>, !fir.ref<!fir.array<10x10xf32>>, !fir.ref<!fir.array<10x10xf32>>) {
 ! CHECK:        acc.yield
 ! CHECK-NEXT: }{{$}}
+! CHECK: acc.delete accPtr(%[[PRESENT_A]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "a"}
+! CHECK: acc.delete accPtr(%[[PRESENT_B]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "b"}
+! CHECK: acc.delete accPtr(%[[PRESENT_C]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "c"}
 
   !$acc parallel deviceptr(a) deviceptr(c)
   !$acc end parallel
diff --git a/flang/test/Lower/OpenACC/acc-serial-loop.f90 b/flang/test/Lower/OpenACC/acc-serial-loop.f90
index 66cde19ef09af41..a94f0e8e8583eb9 100644
--- a/flang/test/Lower/OpenACC/acc-serial-loop.f90
+++ b/flang/test/Lower/OpenACC/acc-serial-loop.f90
@@ -334,6 +334,8 @@ subroutine acc_serial_loop
 ! CHECK-NEXT:   }{{$}}
 ! CHECK:        acc.yield
 ! CHECK-NEXT: }{{$}}
+! CHECK:      acc.delete accPtr(%[[NOCREATE_A]] : !fir.ref<!fir.array<10xf32>>) {dataClause = #acc<data_clause acc_no_create>, name = "a"}
+! CHECK:      acc.delete accPtr(%[[NOCREATE_B]] : !fir.ref<!fir.array<10xf32>>) {dataClause = #acc<data_clause acc_no_create>, name = "b"}
 
   !$acc serial loop present(a, b)
   DO i = 1, n
@@ -348,6 +350,8 @@ subroutine acc_serial_loop
 ! CHECK-NEXT:   }{{$}}
 ! CHECK:        acc.yield
 ! CHECK-NEXT: }{{$}}
+! CHECK:      acc.delete accPtr(%[[PRESENT_A]] : !fir.ref<!fir.array<10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "a"}
+! CHECK:      acc.delete accPtr(%[[PRESENT_B]] : !fir.ref<!fir.array<10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "b"}
 
   !$acc serial loop deviceptr(a) deviceptr(b)
   DO i = 1, n
diff --git a/flang/test/Lower/OpenACC/acc-serial.f90 b/flang/test/Lower/OpenACC/acc-serial.f90
index 88b801a82ee6818..f2f73876af87775 100644
--- a/flang/test/Lower/OpenACC/acc-serial.f90
+++ b/flang/test/Lower/OpenACC/acc-serial.f90
@@ -227,6 +227,9 @@ subroutine acc_serial
 ! CHECK:      acc.serial dataOperands(%[[NO_CREATE_A]], %[[NO_CREATE_B]], %[[CREATE_C]] : !fir.ref<!fir.array<10x10xf32>>, !fir.ref<!fir.array<10x10xf32>>, !fir.ref<!fir.array<10x10xf32>>) {
 ! CHECK:        acc.yield
 ! CHECK-NEXT: }{{$}}
+! CHECK: acc.delete accPtr(%[[CREATE_C]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_create_zero>, name = "c"}
+! CHECK: acc.delete accPtr(%[[NO_CREATE_A]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_no_create>, name = "a"}
+! CHECK: acc.delete accPtr(%[[NO_CREATE_B]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_no_create>, name = "b"}
 
   !$acc serial present(a, b, c)
   !$acc end serial
@@ -237,6 +240,9 @@ subroutine acc_serial
 ! CHECK:      acc.serial dataOperands(%[[PRESENT_A]], %[[PRESENT_B]], %[[PRESENT_C]] : !fir.ref<!fir.array<10x10xf32>>, !fir.ref<!fir.array<10x10xf32>>, !fir.ref<!fir.array<10x10xf32>>) {
 ! CHECK:        acc.yield
 ! CHECK-NEXT: }{{$}}
+! CHECK: acc.delete accPtr(%[[PRESENT_A]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "a"}
+! CHECK: acc.delete accPtr(%[[PRESENT_B]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "b"}
+! CHECK: acc.delete accPtr(%[[PRESENT_C]] : !fir.ref<!fir.array<10x10xf32>>) {dataClause = #acc<data_clause acc_present>, name = "c"}
 
   !$acc serial deviceptr(a) deviceptr(c)
   !$acc end serial
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index aaa3db22110ac72..8f150cb33e6f3c7 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -580,6 +580,7 @@ LogicalResult acc::DeleteOp::verify() {
       getDataClause() != acc::DataClause::acc_copyin &&
       getDataClause() != acc::DataClause::acc_copyin_readonly &&
       getDataClause() != acc::DataClause::acc_present &&
+      getDataClause() != acc::DataClause::acc_no_create &&
       getDataClause() != acc::DataClause::acc_declare_device_resident &&
       getDataClause() != acc::DataClause::acc_declare_link)
     return emitError(

github-actions · 2025-02-10T18:03:14Z

✅ With the latest revision this PR passed the C/C++ code formatter.

khaki3

Nice!

(For comparison, here is the same change to copyin.
f3d3ec8)

razvanlupusoru · 2025-02-10T18:36:02Z

(For comparison, here is the same change to copyin. f3d3ec8)

Thank you for this comment - it made me realize I forgot to handle the implicit declare region support for present. I am going to address it in a few minutes. :)

…ate (llvm#126560) The acc.delete operation has semantics of decrementing present counter and deleting the data when the counter reaches zero. Since both acc.present and acc.nocreate are both intended to increment present counter, this matching exit action must be inserted. This is also what was specified in OpenACC dialect documentation: https://mlir.llvm.org/docs/Dialects/OpenACCDialect/#operation-categories

razvanlupusoru requested review from clementval, khaki3 and vzakhari February 10, 2025 17:59

llvmbot added mlir flang Flang issues not falling into any other category mlir:openacc flang:fir-hlfir openacc labels Feb 10, 2025

Fix formatting

fed14e2

khaki3 approved these changes Feb 10, 2025

View reviewed changes

Add support for exit action for present used in acc declare

01ba4d3

razvanlupusoru merged commit 83edbd4 into llvm:main Feb 10, 2025
8 checks passed

khaki3 mentioned this pull request Jun 18, 2025

[flang][acc] Generate acc.copyout for the reduction clause on compute constructs #144623

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang][acc] Ensure data exit action is generated for present & nocreate #126560

[flang][acc] Ensure data exit action is generated for present & nocreate #126560

Uh oh!

razvanlupusoru commented Feb 10, 2025

Uh oh!

llvmbot commented Feb 10, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Feb 10, 2025 •

edited

Loading

Uh oh!

khaki3 left a comment

Uh oh!

razvanlupusoru commented Feb 10, 2025

Uh oh!

Uh oh!

Uh oh!

[flang][acc] Ensure data exit action is generated for present & nocreate #126560

[flang][acc] Ensure data exit action is generated for present & nocreate #126560

Uh oh!

Conversation

razvanlupusoru commented Feb 10, 2025

Uh oh!

llvmbot commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

khaki3 left a comment

Choose a reason for hiding this comment

Uh oh!

razvanlupusoru commented Feb 10, 2025

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Feb 10, 2025 •

edited

Loading

github-actions bot commented Feb 10, 2025 •

edited

Loading