[PATCH] D79526: [CUDA][HIP] Workaround for resolving host device function against wrong-sided function

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 11 12:23:21 PDT 2020


tra added inline comments.


================
Comment at: clang/include/clang/Sema/Sema.h:11670
 
+  bool IsCUDAImplicitHostDeviceFunction(const FunctionDecl *D);
+
----------------
I think this can be `static` as it does not need Sema's state.


================
Comment at: clang/lib/Sema/SemaCUDA.cpp:217-220
+  if (auto *A = D->getAttr<AttrT>())
+    if (A->isImplicit())
+      return true;
+  return D->isImplicit();
----------------
Is it possible for us to ever end up here with an explicitly set attribute but with an implicit function? If that were to happen, we'd return true and that would be incorrect.
Perhaps add an assert to make sure it does not happen or always return `A->isImplicit()` if an attribute is already set.


================
Comment at: clang/test/SemaCUDA/function-overload.cu:471-477
+inline double callee(double x);
+#pragma clang force_cuda_host_device begin
+inline void callee(int x);
+inline double implicit_hd_caller() {
+  return callee(1.0);
+}
+#pragma clang force_cuda_host_device end
----------------
yaxunl wrote:
> tra wrote:
> > These tests only veryfy that the code compiled, but it does not guarantee that we've picked the correct overload.
> > You should give callees different return types and assign the result to a variable of intended type.  See `test_host_device_calls_hd_template() ` on line 341 for an example.
> they have different return types. The right one returns double and the wrong one returns void. If the wrong one is chosen, there is syntax error since the caller returns double.
Ah. I've missed it. Could you change the types to `struct CorrectOverloadRetTy`/`struct IncorrectOverloadRetTy` to make it more obvious?


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

https://reviews.llvm.org/D79526





More information about the cfe-commits mailing list