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

Marina Yatsina via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 5 11:39:46 PST 2016


myatsina added a comment.

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.
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


https://reviews.llvm.org/D27323





More information about the llvm-commits mailing list