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

[John McCall via Phabricator via cfe-commits]

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.
+  //
----------------
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?


================
Comment at: clang/lib/Sema/SemaOverload.cpp:9749
+  if (isBetterMultiversionCandidate(Cand1, Cand2))
+    return true;
+
----------------
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.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:9752
+  // If other rules cannot determine which is better, CUDA preference is used
+  // to determine which is better.
+  if (S.getLangOpts().CUDA && Cand1.Function && Cand2.Function) {
----------------
Okay, let's think about the right place to put this check in the ordering; we don't want different extensions to get into a who-comes-last competition.

- Certainly this should have lower priority than the standard-defined preferences like argument conversion ranks or `enable_if` partial-ordering.
- The preference for pass-object-size parameters is probably most similar to a type-based-overloading decision and so should take priority.
- I would say that this should take priority over function multi-versioning.  Function multi-versioning is all about making specialized versions of the "same function", whereas I think host/device overloading is meant to be semantically broader than that.

What do you think?

Regardless, the rationale for the order should be explained in comments.


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

https://reviews.llvm.org/D77954