tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.
LGTM with a couple of nits.
================
Comment at: clang/include/clang/Basic/Builtins.h:39
OMP_LANG = 0x80, // builtin requires OpenMP.
+ HIP_LANG = 0x100, // builtin requires HIP.
ALL_LANGUAGES = C_LANG | CXX_LANG | OBJC_LANG, // builtin for all languages.
----------------
The issue is common for both CUDA and HIP.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8310
+def err_hip_invalid_args_builtin_mangled_name : Error<
+ "invalid argument: expect a device-side function or global variable">;
+
----------------
Nit. I'd rephrase it in terms of what makes the argument invalid. E.g. `symbol
must be a device-side function or global variable`.
================
Comment at: clang/lib/Basic/Builtins.cpp:78
bool OpenMPUnsupported = !LangOpts.OpenMP && BuiltinInfo.Langs == OMP_LANG;
+ bool HIPUnsupported = !LangOpts.HIP && BuiltinInfo.Langs == HIP_LANG;
bool CPlusPlusUnsupported =
----------------
tra wrote:
> Wow! The density of negations in this function is impressive.
>
>
Please enable it for CUDA, too.
================
Comment at: clang/lib/Basic/Builtins.cpp:78-84
+ bool HIPUnsupported = !LangOpts.HIP && BuiltinInfo.Langs == HIP_LANG;
bool CPlusPlusUnsupported =
!LangOpts.CPlusPlus && BuiltinInfo.Langs == CXX_LANG;
return !BuiltinsUnsupported && !MathBuiltinsUnsupported && !OclCUnsupported
&&
!OclC1Unsupported && !OclC2Unsupported && !OpenMPUnsupported &&
!GnuModeUnsupported && !MSModeUnsupported && !ObjCUnsupported &&
+ !CPlusPlusUnsupported && !HIPUnsupported;
----------------
Wow! The density of negations in this function is impressive.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:1980-1983
+ if (!D->hasAttr<CUDAGlobalAttr>() && !D->hasAttr<CUDADeviceAttr>() &&
+ !D->hasAttr<CUDAConstantAttr>() && !D->hasAttr<HIPManagedAttr>())
+ return false;
+ return true;
----------------
```
return D->hasAttr<CUDAGlobalAttr>() || D->hasAttr<CUDADeviceAttr>() ||
D->hasAttr<CUDAConstantAttr>() || D->hasAttr<HIPManagedAttr>();
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99301/new/
https://reviews.llvm.org/D99301
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
