[PATCH] D138651: [CUDA][HIP] Don't diagnose use for __bf16

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

> Builds were failing because "__bf16" wasn't allowed on the target.

For CUDA/NVPTX we've solved the issue by implementing storage-only support for NVPTX: https://reviews.llvm.org/D136311
It's a bit more work, but I think it would be a better fix long-term for AMDGPU, too.



================
Comment at: clang/lib/AST/ASTContext.cpp:2175
+      if (Target->hasBFloat16Type() &&
+          (!getLangOpts().OpenMP || !getLangOpts().OpenMPIsDevice)) {
         Width = Target->getBFloat16Width();
----------------
Is there a particular reason we're singling out OpenMP here and not HIP/CUDA?




================
Comment at: clang/lib/AST/ASTContext.cpp:2175
+      if (Target->hasBFloat16Type() &&
+          (!getLangOpts().OpenMP || !getLangOpts().OpenMPIsDevice)) {
         Width = Target->getBFloat16Width();
----------------
tra wrote:
> Is there a particular reason we're singling out OpenMP here and not HIP/CUDA?
> 
> 
Nit: I'd use `if (Target->hasBFloat16Type() && !(getLangOpts().OpenMP && getLangOpts().OpenMPIsDevice))` 



================
Comment at: clang/lib/Sema/SemaType.cpp:1521
   case DeclSpec::TST_BFloat16:
-    if (!S.Context.getTargetInfo().hasBFloat16Type())
+    // Likewise, CUDA host and device may have different __bf16 support.
+    if (!S.Context.getTargetInfo().hasBFloat16Type() && !S.getLangOpts().CUDA &&
----------------
CUDA/NVPTX does have __bf16 support since https://reviews.llvm.org/rG0e8a414ab3d330ebb2996ec95d8141618ee0278b

On the other hand, OpenMP is special-cased, but is not mentioned.


================
Comment at: clang/test/SemaCUDA/amdgpu-bf16.cu:1
+// RUN: %clang_cc1 -fsyntax-only -triple amdgcn-amd-amdhsa -aux-triple x86_64-unknown-linux-gnu -verify %s
+// expected-no-diagnostics
----------------
The changes do affect OpenMP, but there are no OpenMP tests. I think we could use some.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138651