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



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

Reply via email to