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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to