[PATCH] D32416: [x86, SSE] AVX1 PR28129

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

This looks almost done. I have one question about -Os vs. -Oz, and one nit about a code comment.



================
Comment at: lib/Target/X86/X86InstrSSE.td:489
                        [(set VR128:$dst, (v4i32 immAllOnesV))]>;
+  let Predicates = [HasAVX1Only, OptForSize] in
+  def AVX1_SETALLONES: I<0, Pseudo, (outs VR256:$dst), (ins), "",
----------------
Should this be "OptForMinSize"? Ie, I was just looking at some other code patterns, and we are inserting xorps/xorpd even at -Os. 

In fact, we're doing that even at -Oz which I think is a mistake.


================
Comment at: lib/Target/X86/X86InstrSSE.td:7760-7761
 
-
-// Without AVX2 we need to concat two v4i32 V_SETALLONES to create a 256-bit
-// all ones value.
+// To create a 256-bit all ones value, we should produce VCMPTRUEPS
+// with YMM register containing zero.
 let Predicates = [HasAVX1Only] in
----------------
I'd add a little more explanation here just to make it clearer:
"To create a 256-bit all ones value, we use VCMPTRUEPS and zero out the fake register operand to avoid false dependencies."


https://reviews.llvm.org/D32416