[flang-commits] [flang] [Flang][OpenMP] More elegantly handle declare target in unnamed program (PR #95834)

[via flang-commits]

https://github.com/agozillon created https://github.com/llvm/llvm-project/pull/95834

This PR is related to the following issue:

https://github.com/llvm/llvm-project/issues/63362

It tries to solve the crash (which is now slightly different, since the issue has been languishing for a while sorry about that I missed the original issue ping).

The crash occurs due to trying to access the symbol of an undefined/unnamed main when trying to find a declare target symbol that has not been specified (but can be assumed based on it's residence in a function or interface).

The solution in this PR will check if we're trying to retrieve a main symbol, and then if that is the case, we make sure it exists (due to being named) before we attempt to retrieve it, this avoids the crash.

However, that's only part of the issue in the above example, the other is the significant amount of nested directives, I think we are still a little while away from handling this, I have added a reduced variation of the test in the issue as a replicator which contains a lesser number of nesting directives. To push the issue along further, it will likely be a case of working through a number of variations of nested directives in conjunction with target + parallel.

However, this PR pushes the issue above to the point where the issue encountered is identical to the following: https://github.com/llvm/llvm-project/issues/67231

>From fb4cb0f1755871addac02db936fa87540542d087 Mon Sep 17 00:00:00 2001
From: agozillon <Andrew.Gozillon at amd.com>
Date: Mon, 17 Jun 2024 14:45:43 -0500
Subject: [PATCH] [Flang][OpenMP] More elegantly handle declare target in
 unnamed program

This PR is related to the following issue:

https://github.com/llvm/llvm-project/issues/63362

It tries to solve the crash (which is now slightly different, since the
issue has been languishing for a while sorry about that I missed
the original issue ping).

The crash occurs due to trying to access the symbol of an undefined/unnamed main when trying to find a declare
target symbol that has not been specified (but can be
assumed based on it's residence in a function or interface).

The solution in this PR will check if we're trying to retrieve
a main symbol, and then if that is the case, we make sure
it exists (due to being named) before we attempt to retrieve
it, this avoids the crash.

However, that's only part of the issue in the above example,
the other is the significant amount of nested directives, I
think we are still a little while away from handling this, I
have added a reduced variation of the test in the issue
as a replicator which contains a lesser number of nesting
directives. To push the issue along further, it will likely be
a case of working through a number of variations of
nested directives in conjunction with target + parallel.

However, this PR pushes the issue above to the point where
the issue encountered is identical to the following:
https://github.com/llvm/llvm-project/issues/67231
---
 flang/lib/Lower/OpenMP/OpenMP.cpp             |  4 +-
 .../OpenMP/declare-target-unnamed-main.f90    | 41 +++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)
 create mode 100644 flang/test/Lower/OpenMP/declare-target-unnamed-main.f90

diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index aac22f0faad37..3dee2773d29fb 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -296,7 +296,9 @@ static void getDeclareTargetInfo(
   } else if (const auto *clauseList{
                  parser::Unwrap<parser::OmpClauseList>(spec.u)}) {
     List<Clause> clauses = makeClauses(*clauseList, semaCtx);
-    if (clauses.empty()) {
+    if (clauses.empty() &&
+        (!eval.getOwningProcedure()->isMainProgram() ||
+         eval.getOwningProcedure()->getMainProgramSymbol())) {
       // Case: declare target, implicit capture of function
       symbolAndClause.emplace_back(
           mlir::omp::DeclareTargetCaptureClause::to,
diff --git a/flang/test/Lower/OpenMP/declare-target-unnamed-main.f90 b/flang/test/Lower/OpenMP/declare-target-unnamed-main.f90
new file mode 100644
index 0000000000000..b7d6d2fa232ad
--- /dev/null
+++ b/flang/test/Lower/OpenMP/declare-target-unnamed-main.f90
@@ -0,0 +1,41 @@
+!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+!RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s
+
+! This test is a reduced version of the example in issue 63362.
+! It aims to test that no crash occurs when declare target is
+! utilised within an unnamed main program and that we still
+! appropriately mark the function as declare target, even when
+! unused within the target region.
+
+!CHECK: func.func @_QPfoo(%{{.*}}: !fir.ref<f32>{{.*}}) -> f32 attributes {{{.*}}omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to)>{{.*}}}
+
+interface
+real function foo (x)
+  !$omp declare target
+  real, intent(in) :: x
+end function foo
+end interface
+integer, parameter :: n = 1000
+integer, parameter :: c = 100
+integer :: i, j
+real :: a(n)
+do i = 1, n
+a(i) = i
+end do
+do i = 1, n, c
+  !$omp target map(a(i:i+c-1))
+    !$omp parallel do
+      do j = i, i + c - 1
+        a(j) = a(j)
+      end do
+  !$omp end target
+end do
+do i = 1, n
+if (a(i) /= i + 1) stop 1
+end do
+end
+real function foo (x)
+!$omp declare target
+real, intent(in) :: x
+foo = x + 1
+end function foo