[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

[John McCall via Phabricator via cfe-commits]

rjmccall added a reviewer: echristo.
rjmccall added a subscriber: echristo.
rjmccall added inline comments.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:9481
+  // emitted, Cand1 is not better than Cand2. This rule should have precedence
+  // over other rules.
+  //
----------------
yaxunl wrote:
> rjmccall wrote:
> > Please add `[CUDA]` or something similar to the top of this comment so that readers can immediately know that it's dialect-specific.
> > 
> > At a high level, this part of the rule is essentially saying that CUDA non-emittability is a kind of non-viability.  Should we just make non-emittable functions get flagged as non-viable (which will avoid a lot of relatively expensive conversion checking), or is it important to be able to select non-emittable candidates over candidates that are non-viable for other reasons?
> There are two situations for "bad" callees:
> 
> 1. the callee should never be called. It is not just invalid call in codegen, but also invalid call in AST. e.g. a host function call a device function. In CUDA call preference, it is termed "never". And clang already removed such callees from overload candidates.
> 
> 2. the callee should not be called in codegen, but may be called in AST. This happens with `__host__ __device__` functions when calling a "wrong sided" function. e.g. in device compilation, a `__host__ __device__` function calls a `__host__` function. This is valid in AST since the `__host__ __device__` function may be an inline function which is only called by a `__host__` function. There is a deferred diagnostic for the wrong-sided call, which is triggered only if the caller is emitted. However in overloading resolution, if no better candidates are available, wrong-sided candidates are still viable.
Oh, I see what you're saying; sorry, I mis-read the code.  So anything with a preference *worse* than wrong-sided is outright non-viable; there's a very strong preference against wrong-sided calls that takes priority of all of the normal overload-resolution rules; and then there's a very weak preference against non-exact matches that everything else takes priority over.  Okay.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:9749
+  if (isBetterMultiversionCandidate(Cand1, Cand2))
+    return true;
+
----------------
yaxunl wrote:
> rjmccall wrote:
> > If we move anything below this check, it needs to figure out a tri-state so that it can return false if `Cand2` is a better candidate than `Cand1`.  Now, that only matters if multiversion functions are supported under CUDA, but if you're relying on them not being supported, that should at least be commented on.
> multiversion host functions is orthogonal to CUDA therefore should be supported. multiversion in device, host device, and global functions are not supported. However this change does not make things worse, and should continue to work if they are supported.
> 
> host/device based overloading resolution is mostly for determining viability of a function. If two functions are both viable, other factors should take precedence in preference. This general rule has been taken for cases other than multiversion, I think it should also apply to multiversion.
> 
> I will make isBetterMultiversionCandidate three states.
> This general rule has been taken for cases other than multiversion, I think it should also apply to multiversion.

Well, but the multiversion people could say the same: that multiversioning is for picking an alternative among otherwise-identical functions, and HD and H functions are not otherwise-identical.

CC'ing @echristo for his thoughts on the right ordering here.


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

https://reviews.llvm.org/D77954