[PATCH] D99301: [HIP] add __builtin_get_device_side_mangled_name

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 24 15:24:45 PDT 2021


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



More information about the cfe-commits mailing list