Skip to content
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

Fix threadprivate variable scope inside BLOCK construct. #88921

Merged
merged 2 commits into from
Apr 17, 2024

Conversation

harishch4
Copy link
Contributor

When a local variable inside a BLOCK construct is used as threadprivate variable, llvm-flang throws below error:

error: The THREADPRIVATE directive and the common block or variable in it must appear in the same declaration section of a scoping unit

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 16, 2024

@llvm/pr-subscribers-flang-semantics

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

Author: None (harishch4)

Changes

When a local variable inside a BLOCK construct is used as threadprivate variable, llvm-flang throws below error:

> error: The THREADPRIVATE directive and the common block or variable in it must appear in the same declaration section of a scoping unit


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

2 Files Affected:

  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+1-1)
  • (modified) flang/test/Lower/OpenMP/threadprivate-hlfir.f90 (+19-2)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index e85d8d1f7ab533..bafa242a79302a 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1048,7 +1048,7 @@ void OmpStructureChecker::CheckThreadprivateOrDeclareTargetVar(
                       name->symbol->GetUltimate().owner();
                   if (!curScope.IsTopLevel()) {
                     const semantics::Scope &declScope =
-                        GetProgramUnitContaining(curScope);
+                        GetProgramUnitOrBlockConstructContaining(curScope);
                     const semantics::Symbol *sym{
                         declScope.parent().FindSymbol(name->symbol->name())};
                     if (sym &&
diff --git a/flang/test/Lower/OpenMP/threadprivate-hlfir.f90 b/flang/test/Lower/OpenMP/threadprivate-hlfir.f90
index d39ae1e7011838..f02819ce625de6 100644
--- a/flang/test/Lower/OpenMP/threadprivate-hlfir.f90
+++ b/flang/test/Lower/OpenMP/threadprivate-hlfir.f90
@@ -15,8 +15,6 @@
 !CHECK:      %{{.*}} = fir.call @_FortranAioOutputInteger32(%{{.*}}, %[[TP_VAL]]) fastmath<contract> : (!fir.ref<i8>, i32) -> i1
 !CHECK:      omp.terminator
 
-!CHECK:  fir.global internal @_QFsubEa : i32
-
 subroutine sub()
   integer, save:: a
   !$omp threadprivate(a)
@@ -24,3 +22,22 @@ subroutine sub()
     print *, a
   !$omp end parallel
 end subroutine
+
+!CHECK-LABEL: @_QPsub2() {
+!CHECK:   %[[STACK:.*]] = fir.call @llvm.stacksave.p0() fastmath<contract> : () -> !fir.ref<i8>
+!CHECK:   %[[A:.*]] = fir.address_of(@_QFsub2B1Ea) : !fir.ref<i32>
+!CHECK:   %[[A_DECL:.*]]:2 = hlfir.declare %[[A]] {uniq_name = "_QFsub2B1Ea"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+!CHECK:   %[[TP_A:.*]] = omp.threadprivate %[[A_DECL]]#1 : !fir.ref<i32> -> !fir.ref<i32>
+!CHECK:   %[[TP_A_DECL:.*]]:2 = hlfir.declare %[[TP_A]] {uniq_name = "_QFsub2B1Ea"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+!CHECK:   fir.call @llvm.stackrestore.p0(%[[STACK]]) fastmath<contract> : (!fir.ref<i8>) -> ()
+
+subroutine sub2()
+  BLOCK
+    integer, save :: a
+    !$omp threadprivate(a)
+  END BLOCK
+end subroutine
+
+!CHECK:  fir.global internal @_QFsubEa : i32
+!CHECK:  fir.global internal @_QFsub2B1Ea : i32
+

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG.

Please change the test to a test in Semantics (a test that would error without this patch and would not error with this patch), since the fix is there.

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG. Thanks.

@harishch4 harishch4 merged commit fa61f06 into llvm:main Apr 17, 2024
4 checks passed
@harishch4 harishch4 deleted the tp_block_variable branch April 18, 2024 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:openmp flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants