[flang][OpenMP] Add semantic check for target construct

[shraiysh]

This patch adds the following semantic check for target construct

The result of an omp_set_default_device, omp_get_default_device, or
omp_get_num_devices routine called within a target region is
unspecified.

[llvmbot]

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-openmp

Author: Shraiysh (shraiysh)

Changes

This patch adds the following semantic check for target construct

The result of an omp_set_default_device, omp_get_default_device, or
omp_get_num_devices routine called within a target region is
unspecified.

---

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

3 Files Affected:

• (modified) flang/lib/Semantics/check-omp-structure.cpp (+15)
• (modified) flang/lib/Semantics/check-omp-structure.h (+2)
• (added) flang/test/Semantics/OpenMP/target03.f90 (+17)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 53bdf57ff8efa5a..9d4c3d12870b84e 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -10,6 +10,7 @@
 #include "definable.h"
 #include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/tools.h"
+#include "llvm/ADT/StringSet.h"
 
 namespace Fortran::semantics {
 
@@ -2653,6 +2654,20 @@ void OmpStructureChecker::Enter(const parser::OmpClause::If &x) {
   }
 }
 
+void OmpStructureChecker::Enter(const parser::Call &c) {
+  const parser::Name *name =
+      std::get_if<parser::Name>(&std::get<parser::ProcedureDesignator>(c.t).u);
+  llvm::StringSet rtlfns{"omp_set_default_device", "omp_get_default_device",
+      "omp_get_num_devices"};
+  if (context_.ShouldWarn(common::UsageWarning::Portability) &&
+      GetContext().directive == llvm::omp::OMPD_target && name &&
+      rtlfns.contains(name->ToString())) {
+    context_.Say("The result of an %s routine called within a TARGET region is "
+                 "unspecified."_port_en_US,
+        parser::ToUpperCaseLetters(name->ToString()));
+  }
+}
+
 void OmpStructureChecker::Enter(const parser::OmpClause::Linear &x) {
   CheckAllowed(llvm::omp::Clause::OMPC_linear);
 
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 90e5c9f19127750..2c351c91ab85121 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -116,6 +116,8 @@ class OmpStructureChecker
   void Enter(const parser::OmpAtomicCapture &);
   void Leave(const parser::OmpAtomic &);
 
+  void Enter(const parser::Call &c);
+
 #define GEN_FLANG_CLAUSE_CHECK_ENTER
 #include "llvm/Frontend/OpenMP/OMP.inc"
 
diff --git a/flang/test/Semantics/OpenMP/target03.f90 b/flang/test/Semantics/OpenMP/target03.f90
new file mode 100644
index 000000000000000..d101dae610b3c72
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/target03.f90
@@ -0,0 +1,17 @@
+! RUN: %flang_fc1 -fopenmp -fdebug-dump-parse-tree -pedantic %s 2>&1 | FileCheck %s
+
+program main
+  use omp_lib
+  integer :: x, y
+contains
+  subroutine foo()
+  !$omp target
+    !CHECK: portability: The result of an OMP_GET_DEFAULT_DEVICE routine called within a TARGET region is unspecified.
+    x = omp_get_default_device()
+    !CHECK: portability: The result of an OMP_GET_NUM_DEVICES routine called within a TARGET region is unspecified.
+    y = omp_get_num_devices()
+    !CHECK: portability: The result of an OMP_SET_DEFAULT_DEVICE routine called within a TARGET region is unspecified.
+    call omp_set_default_device(x)
+  !$omp end target
+  end subroutine
+end program

[kiranchandramohan]

This would not work if it is inside a different function right?

program main
  use omp_lib
  integer :: x, y
contains
  subroutine foo()
  !$omp target
  call tmp()
  !$omp end target
  end subroutine
  subroutine tmp()
    x = omp_get_default_device()
    y = omp_get_num_devices()
    call omp_set_default_device(x)
  end subroutine
end program

[shraiysh]

Yes, it will not work in that case. I don't know how to handle it there - should we keep track of which functions call these runtime functions while setting attributes?

[shraiysh]

By the way, clang doesn't report these errors at all, and gcc only reports them if they are in the same function (same as current patch).

[NimishMishra]

Same with gfortran- It gives warning for neither. Is making this warning work for different functions a blocker for us?

@shraiysh Maybe we can discuss further. But at an initial level, I am not able to see how we could do this in semantics at least. Or that if we even have the information to do so. I was mainly thinking in terms of scopes or the details attached to symbols x/y. But doesn't seem like we have relevant information there.

Keeping a track of functions may help. But then we have to also mark which functions exist within target region in some callee function in the entire program. Is that a big ask while we are doing semantic checks?

[NimishMishra]

Could a vector of std::string work here? Not a hard comment from me though.

[shraiysh]

I'll do it in the next version of this patch

[shraiysh]

I think it would be a bit tricky to solve it for general case. Consider

subroutine foo()
  !$omp target
    call bar()
  !$omp end target
end subroutine foo
subroutine bar()
  call baz()
end subroutine bar()
subroutine baz()
  omp_set_default_device(2)
end subroutine baz()

We might need to do some sort of call graph traversal to accurately put this semantic check. Is it ok @kiranchandramohan if we do not handle this case?

[kiranchandramohan]

In general, we have not implemented those semantic checks that cannot be fully handled. Do you have a strong requirement for this check?

[shraiysh]

In general, we have not implemented those semantic checks that cannot be fully handled. Do you have a strong requirement for this check?

No, I do not have a strong requirement for this check. I did not know that generally we have not implemented checks that cannot be fully handled. In that case, I will close this PR. Thank you! :)