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

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: None (agozillon)

<details>
<summary>Changes</summary>

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

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


2 Files Affected:

- (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+3-1) 
- (added) flang/test/Lower/OpenMP/declare-target-unnamed-main.f90 (+41) 


``````````diff
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

``````````

</details>


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