[flang-commits] [flang] abdd453 - [flang][openacc] fix bugs with default(none) checking (#149220)

[via flang-commits]

Author: Andre Kuhlenschmidt
Date: 2025-07-18T13:50:09-07:00
New Revision: abdd4536ce0fc75c7a4ddcc1da5913ec5e028091

URL: https://github.com/llvm/llvm-project/commit/abdd4536ce0fc75c7a4ddcc1da5913ec5e028091
DIFF: https://github.com/llvm/llvm-project/commit/abdd4536ce0fc75c7a4ddcc1da5913ec5e028091.diff

LOG: [flang][openacc] fix bugs with default(none) checking (#149220)

A report of the following code not generating an error led to fixing two bugs in directive checking.

- We should treat CombinedConstructs as OpenACC Constructs
- We should treat DoConstruct index variables as private. 

```fortran
subroutine sub(nn)
  integer :: nn, ii
  !$acc serial loop default(none)
  do ii = 1, nn
  end do
  !$acc end serial loop
end subroutine
```
Here `nn` should be flagged as needing a data clause while `ii` should
still get one implicitly.

Added: 
    

Modified: 
    flang/lib/Semantics/resolve-directives.cpp
    flang/test/Lower/OpenACC/acc-loop.f90
    flang/test/Semantics/OpenACC/acc-kernels-loop.f90

Removed: 
    


################################################################################
diff  --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 151f4ccae634e..521c7432d9fbb 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -138,6 +138,9 @@ class AccAttributeVisitor : DirectiveAttributeVisitor<llvm::acc::Directive> {
   void Post(const parser::OpenACCBlockConstruct &) { PopContext(); }
   bool Pre(const parser::OpenACCCombinedConstruct &);
   void Post(const parser::OpenACCCombinedConstruct &) { PopContext(); }
+  void Post(const parser::AccBeginCombinedDirective &) {
+    GetContext().withinConstruct = true;
+  }
 
   bool Pre(const parser::OpenACCDeclarativeConstruct &);
   void Post(const parser::OpenACCDeclarativeConstruct &) { PopContext(); }
@@ -160,6 +163,18 @@ class AccAttributeVisitor : DirectiveAttributeVisitor<llvm::acc::Directive> {
     GetContext().withinConstruct = true;
   }
 
+  // TODO: We should probably also privatize ConcurrentBounds.
+  template <typename A>
+  bool Pre(const parser::LoopBounds<parser::ScalarName, A> &x) {
+    if (!dirContext_.empty() && GetContext().withinConstruct) {
+      if (auto *symbol{ResolveAcc(
+              x.name.thing, Symbol::Flag::AccPrivate, currScope())}) {
+        AddToContextObjectWithDSA(*symbol, Symbol::Flag::AccPrivate);
+      }
+    }
+    return true;
+  }
+
   bool Pre(const parser::OpenACCStandaloneConstruct &);
   void Post(const parser::OpenACCStandaloneConstruct &) { PopContext(); }
   void Post(const parser::AccStandaloneDirective &) {

diff  --git a/flang/test/Lower/OpenACC/acc-loop.f90 b/flang/test/Lower/OpenACC/acc-loop.f90
index c6df28ec5e000..f9f5e8c2165d5 100644
--- a/flang/test/Lower/OpenACC/acc-loop.f90
+++ b/flang/test/Lower/OpenACC/acc-loop.f90
@@ -372,12 +372,15 @@ subroutine sub1(i, j, k)
 end subroutine
 
 ! CHECK: func.func @_QPsub1
-! CHECK: acc.parallel
-! CHECK: %[[DC_K:.*]] = fir.alloca i32 {bindc_name = "k"}
-! CHECK: %[[DC_J:.*]] = fir.alloca i32 {bindc_name = "j"}
-! CHECK: %[[DC_I:.*]] = fir.alloca i32 {bindc_name = "i"}
-! CHECK: %[[P_I:.*]] = acc.private varPtr(%[[DC_I]] : !fir.ref<i32>) -> !fir.ref<i32> {implicit = true, name = "i"}
-! CHECK: %[[P_J:.*]] = acc.private varPtr(%[[DC_J]] : !fir.ref<i32>) -> !fir.ref<i32> {implicit = true, name = "j"}
-! CHECK: %[[P_K:.*]] = acc.private varPtr(%[[DC_K]] : !fir.ref<i32>) -> !fir.ref<i32> {implicit = true, name = "k"}
+! CHECK-SAME: %[[ARG_I:.*]]: !fir.ref<i32> {fir.bindc_name = "i"}
+! CHECK-SAME: %[[ARG_J:.*]]: !fir.ref<i32> {fir.bindc_name = "j"}
+! CHECK-SAME: %[[ARG_K:.*]]: !fir.ref<i32> {fir.bindc_name = "k"}
+! CHECK: %[[DC_I:.*]]:2 = hlfir.declare %[[ARG_I]] dummy_scope %0
+! CHECK: %[[DC_J:.*]]:2 = hlfir.declare %[[ARG_J]] dummy_scope %0 
+! CHECK: %[[DC_K:.*]]:2 = hlfir.declare %[[ARG_K]] dummy_scope %0 
+! CHECK: acc.parallel combined(loop)
+! CHECK: %[[P_I:.*]] = acc.private varPtr(%[[DC_I]]#0 : !fir.ref<i32>) -> !fir.ref<i32> {implicit = true, name = "i"}
+! CHECK: %[[P_J:.*]] = acc.private varPtr(%[[DC_J]]#0 : !fir.ref<i32>) -> !fir.ref<i32> {implicit = true, name = "j"}
+! CHECK: %[[P_K:.*]] = acc.private varPtr(%[[DC_K]]#0 : !fir.ref<i32>) -> !fir.ref<i32> {implicit = true, name = "k"}
 ! CHECK: acc.loop combined(parallel) private(@privatization_ref_i32 -> %[[P_I]] : !fir.ref<i32>, @privatization_ref_i32 -> %[[P_J]] : !fir.ref<i32>, @privatization_ref_i32 -> %[[P_K]] : !fir.ref<i32>) control(%{{.*}} : i32, %{{.*}} : i32, %{{.*}} : i32) = (%c1{{.*}}, %c1{{.*}}, %c1{{.*}} : i32, i32, i32) to (%c10{{.*}}, %c100{{.*}}, %c200{{.*}} : i32, i32, i32)  step (%c1{{.*}}, %c1{{.*}}, %c1{{.*}} : i32, i32, i32)
 ! CHECK: } attributes {inclusiveUpperbound = array<i1: true, true, true>, independent = [#acc.device_type<none>]}

diff  --git a/flang/test/Semantics/OpenACC/acc-kernels-loop.f90 b/flang/test/Semantics/OpenACC/acc-kernels-loop.f90
index 29985a02eb6ef..cfe27e4f8fca1 100644
--- a/flang/test/Semantics/OpenACC/acc-kernels-loop.f90
+++ b/flang/test/Semantics/OpenACC/acc-kernels-loop.f90
@@ -243,8 +243,15 @@ program openacc_kernels_loop_validity
     a(i) = 3.14
   end do
 
+  !$acc kernels loop default(none) private(N, a)
+  do i = 1, N
+    a(i) = 3.14
+  end do
+
   !$acc kernels loop default(none)
+  !ERROR: The DEFAULT(NONE) clause requires that 'n' must be listed in a data-mapping clause
   do i = 1, N
+    !ERROR: The DEFAULT(NONE) clause requires that 'a' must be listed in a data-mapping clause
     a(i) = 3.14
   end do