[PATCH] D95928: [OpenMP] Delay more diagnostics of potentially non-emitted code

Johannes Doerfert via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 15 11:03:11 PST 2021


jdoerfert marked 2 inline comments as done.
jdoerfert added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:18345
   if (LangOpts.OpenMPIsDevice) {
+    // In OpenMP device mode we will not emit host only functions, or functions
+    // we don't need due to their linkage.
----------------
JonChesterfield wrote:
> What catches a static function whose address is taken?
I don't know where but the existing logic in `GetGVALinkageForFunction` seems to handle this just fine. I marked the tests I introduced for such cases below.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:18376
+
+  if (Final && LangOpts.OpenMP && !LangOpts.CUDA)
+    return FunctionEmissionStatus::Emitted;
----------------
JonChesterfield wrote:
> Does LangOpts.HIP imply LandOpts.CUDA? I think it is intended to. I think I've seen this pattern (is openmp and is not cuda) elsewhere, perhaps we're missing a predicate.
> 
I don't know. This is a reinterpretation of the conditional we had, which checked this in a different way but the intention is the same. There we also never looked for HIP, IIRC.


================
Comment at: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp:164
+// expected-note at +1 {{called by 'external'}}
   void *p6 = reinterpret_cast<void*>(&ld_use4);
 }
----------------
Here we test static/inline function with taken address.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95928



More information about the cfe-commits mailing list