[flang-commits] [flang] [Flang][OpenMP] Fix to variables not inheriting data sharing attributes (PR #79017)

[via flang-commits]

https://github.com/harishch4 created https://github.com/llvm/llvm-project/pull/79017

When a default(none) clause exists and a threadprivate variable is used inside the construct, the variable does not inherit threadprivate behavior and throws the below error.

> error: The DEFAULT(NONE) clause requires that 'a' must be listed in a data-sharing attribute clause

Added a condition to skip the error if it is a threadprivate variable.

Fixes: https://github.com/llvm/llvm-project/issues/49545

>From 532754c718c0ca2f26bd9ab36bc8514500a4a166 Mon Sep 17 00:00:00 2001
From: Harish Chambeti <harishcse44 at gmail.com>
Date: Mon, 22 Jan 2024 23:01:28 +0530
Subject: [PATCH] [Flang][OpenMP] Fix to variables not inheriting data sharing
 attributes

---
 flang/lib/Semantics/resolve-directives.cpp    |  1 +
 .../OpenMP/threadprivate-default-clause.f90   | 60 +++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 flang/test/Lower/OpenMP/threadprivate-default-clause.f90

diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 2c570bc3abeb20b..e19f68eefa28672 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1954,6 +1954,7 @@ void OmpAttributeVisitor::Post(const parser::Name &name) {
         if (symbol != found) {
           name.symbol = found; // adjust the symbol within region
         } else if (GetContext().defaultDSA == Symbol::Flag::OmpNone &&
+            !symbol->test(Symbol::Flag::OmpThreadprivate) &&
             // Exclude indices of sequential loops that are privatised in
             // the scope of the parallel region, and not in this scope.
             // TODO: check whether this should be caught in IsObjectWithDSA
diff --git a/flang/test/Lower/OpenMP/threadprivate-default-clause.f90 b/flang/test/Lower/OpenMP/threadprivate-default-clause.f90
new file mode 100644
index 000000000000000..be07aeca2d5c32a
--- /dev/null
+++ b/flang/test/Lower/OpenMP/threadprivate-default-clause.f90
@@ -0,0 +1,60 @@
+! Simple test for lowering of OpenMP Threadprivate Directive with HLFIR.
+
+!RUN: %flang_fc1  -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+!CHECK-LABEL: func.func @_QPsub1() {
+!CHECK:     %[[A:.*]] = fir.address_of(@_QFsub1Ea) : !fir.ref<i32>
+!CHECK:     %[[A_DECL:.*]]:2 = hlfir.declare %[[A]]  {uniq_name = "_QFsub1Ea"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+!CHECK:     %[[A_TP0:.*]] = omp.threadprivate %[[A_DECL]]#1 : !fir.ref<i32> -> !fir.ref<i32>
+!CHECK:     %[[A_CVT:.*]]:2 = hlfir.declare %[[A_TP0]] {uniq_name = "_QFsub1Ea"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+!CHECK:     omp.parallel {
+!CHECK:       %[[A_TP:.*]] = omp.threadprivate %[[A_DECL]]#1 : !fir.ref<i32> -> !fir.ref<i32>
+!CHECK:       %[[A_TP_DECL:.*]]:2 = hlfir.declare %[[A_TP]] {uniq_name = "_QFsub1Ea"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+!CHECK:       %[[TID:.*]] = fir.call @omp_get_thread_num() fastmath<contract> : () -> i32
+!CHECK:       hlfir.assign %[[TID]] to %[[A_TP_DECL]]#0 : i32, !fir.ref<i32>
+!CHECK:       omp.terminator
+!CHECK:     }
+
+subroutine sub1()
+   use omp_lib
+   integer, save :: a
+   !$omp threadprivate(a)
+   !$omp parallel default(none)
+   a = omp_get_thread_num()
+   !$omp end parallel
+end subroutine
+
+!CHECK-LABEL: func.func @_QPsub2() {
+!CHECK:    %[[BLK:.*]] = fir.address_of(@blk_) : !fir.ref<!fir.array<4xi8>>
+!CHECK:    %[[BLK_CVT:.*]] = fir.convert %[[BLK]] : (!fir.ref<!fir.array<4xi8>>) -> !fir.ref<!fir.array<?xi8>>
+!CHECK:    %c0 = arith.constant 0 : index
+!CHECK:    %[[A_ADDR:.*]] = fir.coordinate_of %[[BLK_CVT]], %c0 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
+!CHECK:    %[[A_CVT:.*]] = fir.convert %[[A_ADDR]] : (!fir.ref<i8>) -> !fir.ref<i32>
+!CHECK:    %[[A_DECL:.*]]:2 = hlfir.declare %[[A_CVT]] {uniq_name = "_QFsub2Ea"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+!CHECK:    %[[A_TP0:.*]] = omp.threadprivate %[[BLK]] : !fir.ref<!fir.array<4xi8>> -> !fir.ref<!fir.array<4xi8>>
+!CHECK:    %[[A_TP0_CVT:.*]] = fir.convert %[[A_TP0]] : (!fir.ref<!fir.array<4xi8>>) -> !fir.ref<!fir.array<?xi8>>
+!CHECK:    %c0_0 = arith.constant 0 : index
+!CHECK:    %[[A_TP0_ADDR:.*]] = fir.coordinate_of %[[A_TP0_CVT]], %c0_0 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
+!CHECK:    %[[A_TP0_ADDR_CVT:.*]] = fir.convert %[[A_TP0_ADDR]] : (!fir.ref<i8>) -> !fir.ref<i32>
+!CHECK:    %[[A_TP0_DECL:.*]]:2 = hlfir.declare %[[A_TP0_ADDR_CVT]] {uniq_name = "_QFsub2Ea"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+!CHECK:    omp.parallel {
+!CHECK:       %[[BLK_TP:.*]] = omp.threadprivate %[[BLK]] : !fir.ref<!fir.array<4xi8>> -> !fir.ref<!fir.array<4xi8>>
+!CHECK:       %[[BLK_TP_CVT:.*]] = fir.convert %[[BLK_TP]] : (!fir.ref<!fir.array<4xi8>>) -> !fir.ref<!fir.array<?xi8>>
+!CHECK:       %c0_1 = arith.constant 0 : index
+!CHECK:       %[[A_TP_ADDR:.*]] = fir.coordinate_of %[[BLK_TP_CVT]], %c0_1 : (!fir.ref<!fir.array<?xi8>>, index) -> !fir.ref<i8>
+!CHECK:       %[[A_TP_ADDR_CVT:.*]] = fir.convert %[[A_TP_ADDR]] : (!fir.ref<i8>) -> !fir.ref<i32>
+!CHECK:       %[[A_TP_DECL:.*]]:2 = hlfir.declare %[[A_TP_ADDR_CVT]] {uniq_name = "_QFsub2Ea"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+!CHECK:       %[[TID:.*]] = fir.call @omp_get_thread_num() fastmath<contract> : () -> i32
+!CHECK:       hlfir.assign %[[TID]] to %[[A_TP_DECL]]#0 : i32, !fir.ref<i32>
+!CHECK:       omp.terminator
+!CHECK:     }
+
+subroutine sub2()
+   use omp_lib
+   integer :: a
+   common/blk/a
+   !$omp threadprivate(/blk/)
+   !$omp parallel default(none)
+   a = omp_get_thread_num()
+   !$omp end parallel
+end subroutine