[PATCH] D25809: [CUDA] Improved target attribute-based overloading.

[Justin Lebar via cfe-commits]

jlebar accepted this revision.
jlebar added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lib/Sema/SemaCUDA.cpp:87
+
+  if ((HasHostAttr && HasDeviceAttr) || ForceCUDAHostDeviceDepth > 0)
+    return CFT_HostDevice;
----------------
tra wrote:
> jlebar wrote:
> > Checking ForceCUDAHostDeviceDepth here is...yeah.  Especially because the other overload of this function doesn't do it.
> > 
> > I know you get rid of it in the next patch, but how much work would it be to get rid of it here?  It definitely makes this patch harder to check.
> OK. Moved the check to the place where I use it.
Thank you -- I appreciate that.  :)


================
Comment at: lib/Sema/SemaCUDA.cpp:791
+  CUDAFunctionTarget NewTarget = IdentifyCUDATarget(NewFD);
+  for (auto OldND : Previous) {
+    FunctionDecl *OldFD = OldND->getAsFunction();
----------------
tra wrote:
> jlebar wrote:
> > If this is just a Decl* or NamedDecl*, can we write out the type?
> I'm not sure what exactly you'd like to see. Diags will print out the line and target info for both sides.
> Could you give me example of existing diagnostics that would be similar to what you want?
Sorry, I just meant for you to change "auto" to the actual type.


================
Comment at: lib/Sema/SemaCUDA.cpp:793
+    FunctionDecl *OldFD = OldND->getAsFunction();
+    if (!OldFD || OldFD->isFunctionTemplateSpecialization())
+      continue;
----------------
tra wrote:
> jlebar wrote:
> > Please add a comment explaining why we ignore template specializations.
> That's another check that's no longer needed as the only template instantiations/specializations that end up in the overload set are those that were instantiated from templates with matching target.
> 
Even better.  :)


================
Comment at: lib/Sema/SemaTemplate.cpp:7047
+      // target. Given that regular template deduction does not take
+      // it into account, we perform target match check here and
+      // reject candidates that have different target.
----------------
what is "it"?  Do you mean "the function signature", or something else?


https://reviews.llvm.org/D25809