[PATCH] D56411: [CUDA][HIP][Sema] Fix template kernel with function as template parameter

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 15 20:20:17 PST 2019


rjmccall added a comment.

In D56411#1352602 <https://reviews.llvm.org/D56411#1352602>, @yaxunl wrote:

> In D56411#1352332 <https://reviews.llvm.org/D56411#1352332>, @rjmccall wrote:
>
> > This patch still doesn't make any sense.  You don't need to do any special validation when passing a function as a template argument.  When Sema instantiates the template definition, it'll rebuild the expressions that refer to the template parameter, which will trigger the normal checking for whether those expressions are illegally referencing a host function from the device, etc.  All you need to do is suppress that checking (whether it happens in a template definition or not) for references from non-potentially-evaluated contexts.
>
>
> If you look at line 6583 of lib/Sema/SemaTemplate.cpp, you will see clang does the check if the function needs overloading resolution. However, clang missed the check if the function does not need overloading resolution. That's why I need to add the check at line 6593. All the other stuff is just to help make this check.
>
> why clang does not do the reference check when there is no overloading resolution?


We should have already done the check for a non-overloaded function reference as part of building the DRE.  See `Sema::BuildDeclarationNameExpr`.  Template argument checking can resolve an overload set based on the type of the template parameter, so overload sets have to be treated specially there.

> I think in usual cases clang already does that check during template argument parsing, so it does not need to do that again at line 6593. Unfortunately, for CUDA host/device check, it has to be skipped in template argument parsing and deferred to line 6593.

Again, you really should not ever impose this restriction in template arguments.


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

https://reviews.llvm.org/D56411





More information about the cfe-commits mailing list