[Openmp-commits] [PATCH] D95912: [OpenMP] Attribute target diagnostics properly

Mariya Podchishchaeva via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Mon Feb 15 07:45:56 PST 2021

Fznamznon added inline comments.

Comment at: clang/lib/Sema/Sema.cpp:1798
+  FunctionDecl *FD =
+      isa<FunctionDecl>(C) ? cast<FunctionDecl>(C) : dyn_cast<FunctionDecl>(D);
   auto CheckType = [&](QualType Ty) {
So, we assume that lexical context is a function and if it is not, we assume that the value declaration being checked is a function. Can we actually break this assumption?
For example, will something like this work correctly:
struct S {
    __float128 A = 1;
    int bar = 2 * A;
#pragma omp declare target
  S s;
#pragma omp end declare target

Comment at: clang/lib/Sema/Sema.cpp:1835
+  if (const auto *FPTy = dyn_cast<FunctionNoProtoType>(Ty))
+    CheckType(FPTy->getReturnType());
I was a bit confused by `FPTy` variable name and already started writing a comment that we already checked a case `FunctionProtoType` case, then I realized that it is a check for Function*No*Prototype :) . Can we change FPTy to something closer to `FunctionNoProtoType` ?

Comment at: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp:42
 #ifndef _ARCH_PPC
-// expected-note at +1 {{'boo' defined here}}
+// expected-error at +2 {{'boo' requires 128 bit size '__float128' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+// expected-note at +1 2{{'boo' defined here}}
So,  `boo` is diagnosed two times, at the point where it actually used and where it is defined, it looks a bit like a duplication isn't it? I think it makes sense to diagnose only usage point with note pointing to the definition.

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list