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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 6 inline comments as done.
yaxunl added inline comments.


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


================
Comment at: clang/lib/Sema/SemaCUDA.cpp:217-220
+  if (auto *A = D->getAttr<AttrT>())
+    if (A->isImplicit())
+      return true;
+  return D->isImplicit();
----------------
tra wrote:
> 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.
will return A->isImplicit()


================
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
----------------
tra wrote:
> 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?
will do


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

https://reviews.llvm.org/D79526