tra added inline comments.

================
Comment at: clang/lib/Sema/SemaType.cpp:8168
+    IsTargetCUDAAndHostARM =
+        !AuxTI || AuxTI->getTriple().isAArch64() || AuxTI->getTriple().isARM();
+  }
----------------
Should it be `AuxTI && (AuxTI->getTriple().isAArch64() || 
AuxTI->getTriple().isARM();)` ?

I don't think we want IsTargetCUDAAndHostARM to be set to true if there's no 
auxTargetInfo (e.g. during any C++ compilation, regardless of the target).


================
Comment at: clang/lib/Sema/SemaType.cpp:8173-8174
   // not to need a separate attribute)
   if (!S.Context.getTargetInfo().hasFeature("neon") &&
-      !S.Context.getTargetInfo().hasFeature("mve")) {
+      !S.Context.getTargetInfo().hasFeature("mve") && !IsTargetCUDAAndHostARM) 
{
     S.Diag(Attr.getLoc(), diag::err_attribute_unsupported)
----------------
Nit: `!(S.Context.getTargetInfo().hasFeature("neon") || 
S.Context.getTargetInfo().hasFeature("mve") || IsTargetCUDAAndHostARM)` would 
be a bit easier to read.


================
Comment at: clang/test/SemaCUDA/neon-attrs.cu:1
+// RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon -x cuda 
-fsyntax-only -DNO_DIAG -verify %s
+// RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature -neon -x cuda 
-fsyntax-only -verify %s
----------------
Instead of replicating the code, you could use different verify prefix for each 
case.
E.g. `-verify=quiet` and then in the body of the test use `// 
quiet-no-diagnostics`.


================
Comment at: clang/test/SemaCUDA/neon-attrs.cu:2
+// RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature +neon -x cuda 
-fsyntax-only -DNO_DIAG -verify %s
+// RUN: %clang_cc1 -triple arm64-linux-gnu -target-feature -neon -x cuda 
-fsyntax-only -verify %s
+
----------------
You should also pass `-aux-triple nvptx64...`.

This also needs more test cases. This only tests host-side CUDA compilation.
We also need:
```
// GPU-side compilation on ARM (no errors expected)
// RUN: %clang_cc1 -aux-triple arm64-linux-gnu -triple nvptx64 -fcuda-is-device 
 -x cuda -fsyntax-only -DNO_DIAG -verify %s
// Regular C++ compilation on x86 and ARM without neon (should produce 
diagnostics) 
// RUN: %clang_cc1  -triple x86.... -x c++ -fsyntax-only -verify %s
// RUN: %clang_cc1  -triple arm64... -x c++ -target-feature -neon -fsyntax-only 
-verify %s
// C++ on ARM w/ neon (no diagnostics)
// RUN: %clang_cc1  -triple arm64... -x c++ -target-feature +neon -fsyntax-only 
-DNO_DIAG -verify %s
``` 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152403

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to