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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
