[PATCH] D79526: [CUDA][HIP] Workaround for resolving host device function against wrong-sided function

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D79526#2027242 <https://reviews.llvm.org/D79526#2027242>, @tra wrote:

> The latest version of the patch works well enough to compile tensorflow. That's the good news.
>
> In D79526#2026857 <https://reviews.llvm.org/D79526#2026857>, @yaxunl wrote:
>
> > Looks like we went overboard to treat implicit host device candidate as inferior. They should be treated
> >  as inferior in device compilation, not in host compilation. Here because they are treated as inferior
> >  to same-sided candidate in host compilation, they changed overload resolution in host compilation
> >  therefore caused the failure in host compilation.
> >
> > I have updated the patch to treat implicit host device candidate as inferior in device compilation.
>
>
> I'm concerned that this creates inconsistency in how overload resolution works during host and device compilation.
>  In general they should behave the same. I.e. a test where this change is needed during device-side compilation will require the same change on the host side, if you swap H and D attributes on the functions in the test.
>
> Speaking of tests, it would be great to add a test illustrating this scenario.


I added a test at line 483 for the situation.

For implicit host device functions, since they are not guaranteed to work in device compilation, we can only resolve them as if they are host functions. This causes asymmetry but implicit host device functions are originally host functions so it is biased toward host compilation in the beginning. Only the original resolution guarantees no other issues.  For example, in the failed compilation in TF, some ctor of std::atomic becomes implicit host device function because it is constexpr. We should treated as wrong-sided in device compilation, but we should treated as same-sided in host compilation, otherwise it changes the resolution in host compilation and causes other issues.


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

https://reviews.llvm.org/D79526