[PATCH] D28744: [X86][AVX] Remove "OptForSize" condition from some memory foldings.

Ayman Musa via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 31 01:46:25 PST 2017


aymanmus added inline comments.


================
Comment at: test/CodeGen/X86/avx-arith.ll:353
 ; CHECK:       ## BB#0:
-; CHECK-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
-; CHECK-NEXT:    vsqrtss %xmm0, %xmm0, %xmm0
+; CHECK-NEXT:    vsqrtss (%rax), %xmm0, %xmm0
 ; CHECK-NEXT:    retq
----------------
craig.topper wrote:
> craig.topper wrote:
> > aymanmus wrote:
> > > craig.topper wrote:
> > > > This now has a register read dependency on xmm0 which I believe is what the OptForSize was originally protecting against.  I know work has gone into ExeDepFix for UndefRegClearance. Do we believe that is sufficient to allow this folding now?
> > > The OptForSize was incorrectly copied from the SSE version multiclass, where the memory form instructions performed a partial register updates (optimization guide states that partial updates come with a penalty and thus should be avoided).
> > > I'm afraid I didn't understand how the memory folding adds read dependency on xmm0 in this case, the read dependency was already there.
> > Yes there was a read of xmm0 on the dart but it was from the movss instruction which wrote all bits. After this patch the xmm0 read is dependent on an unknown instruction.
> Oops. Autocorrect turned sqrt into dart.
I see what you mean.
I think this is a general problem that should not be discussed in this patch.
The reason the folding was disabled before has nothing related to this issue. And the problem is not specific for this instruction.
IMHO this patch should not consider this kind of problems as it should be dealt with as a separate manner.


https://reviews.llvm.org/D28744





More information about the llvm-commits mailing list