[PATCH] D27323: [X86] Fix non-intrinsic roundss/roundsd to not read the destination register

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper added a comment.

In https://reviews.llvm.org/D27323#613687, @myatsina wrote:

> In https://reviews.llvm.org/D27323#612970, @mkuper wrote:
>
> > In https://reviews.llvm.org/D27323#612920, @myatsina wrote:
> >
> > > As Michael have already noticed, I also have a solution for this false dependency bug (https://reviews.llvm.org/D27391).
> > >  What happens is that some instructions (like the ROUNDSSr) read an undef value from one of it's source operands.
> > >  Part of the logic that searches for false dependencies decides if the dependency can be broken only if the instruction doesn't read the operand.
> > >  I think that a read of an undef value should not be considered as a real read, and this is the fix in my patch. I believe this approach will catch more cases.
> > >
> > > Thanks,
> > > Marina
> >
> >
> > To reiterate what I wrote on the other patch - the only reason we end up with reading an undef value for the ROUNDSS/Drr is because we have a source operand tied with the destination, and then all patterns that match these instructions shove an IMPLICIT_DEF into that operand - making it, in practice, a dummy operand. This is in contrast to the other SSE instructions that have a false dependence on the high lanes, that simply don't have this operand.
> >
> > I think we should be modeling all these instructions in the same way.
> >  One option is to sync ROUNDSS/Drr with the rest, by removing the operand, like this patch does.
> >  The other is to add the operand to all relevant instructions (making them binary instead of unary), and put something like https://reviews.llvm.org/D27391 in place.
> >  Or am I missing a reason why ROUND should be modeled differently from the rest?
> >
> > Regarding https://reviews.llvm.org/D27391 catching more cases - Marina, do you have anything specific in mind? Which instructions would that apply to?
>
>
> You are right, ROUND shouldn't be modeled differently than the other cases, so your change should go in.


Ok, great, then I'll commit this.

> I found other instructions and intrinsics that look for uses and find uses of undef values:
> 
>   %XMM0<def,tied1> = RCPSSm_Int %XMM0<undef,tied0>, %RDI<kill>, 1, %noreg, 0, %noreg
> 
> So there is a deeper problem here than just ROUND

This looks extremely weird.
(And now I understand what Craig's email was about! :-) )


https://reviews.llvm.org/D27323