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

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

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.



================
Comment at: lib/Target/X86/X86InstrSSE.td:2725
                 !strconcat(OpcodeStr, "\t{$src2, $dst|$dst, $src2}"), []>,
                 Sched<[sched.Folded, ReadAfterLd]>;
   }
----------------
RKSimon wrote:
> Intrinsic version needs updating as well? Although AFAICT this can't be tested within llvm-mca....
I think the intrinsic version is fine because it uses three operands, and ReadAdvance correctly applies to the register operand (i.e. $src1), and not the memory operand.
Also, src1 is tied to $dst.

It would not be easy to test it. As you wrote, we cannot write an llvm-mca since it is a "codegen only" instruction...


https://reviews.llvm.org/D51542