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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
