[PATCH] D67837: [CUDA][HIP] Fix host/device check with -fopenmp

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: lib/Sema/SemaDecl.cpp:17618
+
+  if (LangOpts.CUDA) {
+    // When compiling for device, host functions are never emitted.  Similarly,
----------------
Are you going to handle `#pragma omp declare target device_type(nohost)` in Cuda mode correctly? Such functions should not be emitted on the host in OpenMP 5.0.


================
Comment at: lib/Sema/SemaOpenMP.cpp:1569-1574
+static Sema::FunctionEmissionStatus isKnownDeviceEmitted(Sema &S,
+                                                         FunctionDecl *FD) {
   assert(S.LangOpts.OpenMP && S.LangOpts.OpenMPIsDevice &&
          "Expected OpenMP device compilation.");
-  // Templates are emitted when they're instantiated.
-  if (FD->isDependentContext())
-    return FunctionEmissionStatus::Discarded;
-
-  Optional<OMPDeclareTargetDeclAttr::DevTypeTy> DevTy =
-      OMPDeclareTargetDeclAttr::getDeviceType(FD->getCanonicalDecl());
-  if (DevTy.hasValue())
-    return (*DevTy == OMPDeclareTargetDeclAttr::DT_Host)
-               ? FunctionEmissionStatus::Discarded
-               : FunctionEmissionStatus::Emitted;
-
-  // Otherwise, the function is known-emitted if it's in our set of
-  // known-emitted functions.
-  return (S.DeviceKnownEmittedFns.count(FD) > 0)
-             ? FunctionEmissionStatus::Emitted
-             : FunctionEmissionStatus::Unknown;
+  return S.getEmissionStatus(FD);
 }
----------------
You can remove the whole function and use `Sema::getEmissionStatus()` instead.


================
Comment at: lib/Sema/SemaOpenMP.cpp:1593
+  case FunctionEmissionStatus::OMPDiscarded:
+  case FunctionEmissionStatus::CUDADiscarded:
     Kind = DeviceDiagBuilder::K_Nop;
----------------
I think, `CUDADiscarded` case must be unreachable and it must be a case for `llvm_unreachable()` in case of the OpenMP device code.


================
Comment at: lib/Sema/SemaOpenMP.cpp:1602-1607
+static Sema::FunctionEmissionStatus isKnownHostEmitted(Sema &S,
+                                                       FunctionDecl *FD) {
   assert(S.LangOpts.OpenMP && !S.LangOpts.OpenMPIsDevice &&
          "Expected OpenMP host compilation.");
-  // In OpenMP 4.5 all the functions are host functions.
-  if (S.LangOpts.OpenMP <= 45)
-    return FunctionEmissionStatus::Emitted;
-
-  Optional<OMPDeclareTargetDeclAttr::DevTypeTy> DevTy =
-      OMPDeclareTargetDeclAttr::getDeviceType(FD->getCanonicalDecl());
-  if (DevTy.hasValue())
-    return (*DevTy == OMPDeclareTargetDeclAttr::DT_NoHost)
-               ? FunctionEmissionStatus::Discarded
-               : FunctionEmissionStatus::Emitted;
-
-  // Otherwise, the function is known-emitted if it's in our set of
-  // known-emitted functions.
-  return (S.DeviceKnownEmittedFns.count(FD) > 0)
-             ? FunctionEmissionStatus::Emitted
-             : FunctionEmissionStatus::Unknown;
+  return S.getEmissionStatus(FD);
 }
----------------
You can remove the whole function and use `Sema::getEmissionStatus()` instead.


================
Comment at: lib/Sema/SemaOpenMP.cpp:1647-1648
             FunctionEmissionStatus::Unknown)) &&
       isKnownDeviceEmitted(*this, Callee) ==
-          FunctionEmissionStatus::Discarded) {
+          FunctionEmissionStatus::OMPDiscarded) {
     StringRef HostDevTy =
----------------
I would add an assert thet this function does not return `CUDADiscarded` value.


================
Comment at: lib/Sema/SemaOpenMP.cpp:1684-1685
       isKnownHostEmitted(*this, Caller) == FunctionEmissionStatus::Emitted &&
-      isKnownHostEmitted(*this, Callee) == FunctionEmissionStatus::Discarded) {
+      isKnownHostEmitted(*this, Callee) ==
+          FunctionEmissionStatus::OMPDiscarded) {
     StringRef NoHostDevTy = getOpenMPSimpleClauseTypeName(
----------------
Also, it would be good to check that it cannot return `CUDADiscarded` here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67837/new/

https://reviews.llvm.org/D67837