Skip to content

release/21.x: [flang][OpenMP] Avoid analyzing assumed-size array bases (#150324) #150411

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

Merged
merged 1 commit into from
Jul 28, 2025

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Jul 24, 2025

Backport 8fcbd06

Requested by: @kparzysz

@llvmbot
Copy link
Member Author

llvmbot commented Jul 24, 2025

@tblah What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from tblah July 24, 2025 12:36
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:openmp flang:semantics labels Jul 24, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Jul 24, 2025

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-openmp

Author: None (llvmbot)

Changes

Backport 8fcbd06

Requested by: @kparzysz


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

2 Files Affected:

  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+20-16)
  • (added) flang/test/Semantics/OpenMP/assumed-size-array.f90 (+25)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 2425265e196c6..783aef18fb894 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -16,7 +16,6 @@
 #include "flang/Common/idioms.h"
 #include "flang/Common/indirection.h"
 #include "flang/Common/visit.h"
-#include "flang/Evaluate/shape.h"
 #include "flang/Evaluate/tools.h"
 #include "flang/Evaluate/type.h"
 #include "flang/Parser/char-block.h"
@@ -4117,21 +4116,26 @@ void OmpStructureChecker::CheckArraySection(
   // Detect this by looking for array accesses on character variables which are
   // not arrays.
   bool isSubstring{false};
-  evaluate::ExpressionAnalyzer ea{context_};
-  if (MaybeExpr expr = ea.Analyze(arrayElement.base)) {
-    std::optional<evaluate::Shape> shape = evaluate::GetShape(expr);
-    // Not an array: rank 0
-    if (shape && shape->size() == 0) {
-      if (std::optional<evaluate::DynamicType> type = expr->GetType()) {
-        if (type->category() == evaluate::TypeCategory::Character) {
-          // Substrings are explicitly denied by the standard [6.0:163:9-11].
-          // This is supported as an extension. This restriction was added in
-          // OpenMP 5.2.
-          isSubstring = true;
-          context_.Say(GetContext().clauseSource,
-              "The use of substrings in OpenMP argument lists has been disallowed since OpenMP 5.2."_port_en_US);
-        } else {
-          llvm_unreachable("Array indexing on a variable that isn't an array");
+  // Cannot analyze a base of an assumed-size array on its own. If we know
+  // this is an array (assumed-size or not) we can ignore it, since we're
+  // looking for strings.
+  if (!IsAssumedSizeArray(*name.symbol)) {
+    evaluate::ExpressionAnalyzer ea{context_};
+    if (MaybeExpr expr = ea.Analyze(arrayElement.base)) {
+      if (expr->Rank() == 0) {
+        // Not an array: rank 0
+        if (std::optional<evaluate::DynamicType> type = expr->GetType()) {
+          if (type->category() == evaluate::TypeCategory::Character) {
+            // Substrings are explicitly denied by the standard [6.0:163:9-11].
+            // This is supported as an extension. This restriction was added in
+            // OpenMP 5.2.
+            isSubstring = true;
+            context_.Say(GetContext().clauseSource,
+                "The use of substrings in OpenMP argument lists has been disallowed since OpenMP 5.2."_port_en_US);
+          } else {
+            llvm_unreachable(
+                "Array indexing on a variable that isn't an array");
+          }
         }
       }
     }
diff --git a/flang/test/Semantics/OpenMP/assumed-size-array.f90 b/flang/test/Semantics/OpenMP/assumed-size-array.f90
new file mode 100644
index 0000000000000..6e36db178dc8e
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/assumed-size-array.f90
@@ -0,0 +1,25 @@
+!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+! This should compile without errors. Check for a symptom of a reasonable
+! output.
+
+!CHECK: omp.task depend
+
+subroutine omp_task_depend_reproducer(work, myid, shift)
+  implicit none
+  integer, intent(in) :: myid, shift
+  real, intent(inout) :: work(*)
+
+!$omp parallel shared(work, myid, shift)
+  !$omp single
+    !$omp task depend(in:work(myid+shift-1)) depend(in:work(myid-1)) depend(out:work(myid))
+      call dummy_kernel(work(myid))
+    !$omp end task
+  !$omp end single
+!$omp end parallel
+contains
+  subroutine dummy_kernel(x)
+    real :: x
+    x = x + 1.0
+  end subroutine dummy_kernel
+end subroutine omp_task_depend_reproducer

@github-project-automation github-project-automation bot moved this from Needs Triage to Needs Merge in LLVM Release Status Jul 24, 2025
A check for character substrings masquerading as array sections was
using expression analyzer on the array base. When this array happened to
be an assumed-size array, the analyzer emitted a semantic error that did
not correspond to any issue with the source code.

To avoid that, check whether the object is an assumed-size array before
using the expression analyzer on it.

While at it, replace the call to GetShape with a simple check for rank,
since that's the only information needed.

Fixes llvm#150297

(cherry picked from commit 8fcbd06)
@tru tru merged commit 9554152 into llvm:release/21.x Jul 28, 2025
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status Jul 28, 2025
Copy link

@kparzysz (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:openmp flang:semantics flang Flang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

4 participants