[PATCH] D51542: [X86] Remove wrong ReadAdvance from multiclass sse_fp_unop_s

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D51542#1222120, @andreadb wrote:

> In https://reviews.llvm.org/D51542#1221947, @andreadb wrote:
>
> > In https://reviews.llvm.org/D51542#1221933, @RKSimon wrote:
> >
> > > There seems to be 2 things here - (1) ReadAfterLd is being used incorrectly to try to make a dependency on the dst reg and (2) we have no mechanism to handle partial dependencies like for the scalar sse instructions.
> > >
> > > AFAICT this patch fixes the (1) issue but (2) isn't handled at all by the scheduler models yet.
> >
> >
> > Yep.
> >
> > I will raise a bug for 2. This patch only addresses the issue with ReadAfterLd being incorrectly applied to a register used as the base address of the folded load.
>
>
> Raised bug https://bugs.llvm.org/show_bug.cgi?id=38813.


Thanks! I think everyone agrees that the ReadAfterLd is not behaving as intended on the SSE instructions. I am still wondering about the AVX case (and the SSE intrinsic is the same?):
As noted, this is testing with llvm-mca with skylake CPU:

  With ReadAfterLd:
  
  [0,0]     DeeeeeeeeeER   .   vdppd	$1, %xmm0, %xmm1, %xmm2
  [0,1]     D=eE-------R   .   leaq	8(%rsp,%rdi,2), %rax
  [0,2]     D====eeeeeeeeeER   vrsqrtss	(%rax), %xmm2, %xmm3  <--- execution delayed by vdppd?
  
  No ReadAfterLd:
  
  [0,0]     DeeeeeeeeeER   .    .   vdppd	$1, %xmm0, %xmm1, %xmm2
  [0,1]     D=eE-------R   .    .   leaq	8(%rsp,%rdi,2), %rax
  [0,2]     D=========eeeeeeeeeER   vrsqrtss	(%rax), %xmm2, %xmm3   <--- execution delayed until xmm2 is known

Do we have confirmation from hardware testing or Intel docs that the 1st timeline is correct? We can punt this question to another patch to not delay this one if needed, but I'd like to understand what the hardware does in this case.


https://reviews.llvm.org/D51542