On 23 Mar 12:55, Jakub Jelinek wrote:
> On Wed, Mar 23, 2016 at 02:47:54PM +0300, Kirill Yukhin wrote:
> > > This code is only executed if standard_sse_constant_p returns 2, which
> > > is for 16-byte vectors and all ones for TARGET_SSE2, and for
> > > 32-byte vectors for TARGET_AVX2.
> > > Thus, IMNSHO the patch is wrong, even for plain -mavx -mno-avx2
> > > we want to emit vpcmpeqd %xmm?, %xmm?, %xmm?, so that we don't mix
> > > VEX with non-VEX encoded insns.
> > You're right. I've updated the patch to check mode size as well.
>
> That is also wrong, you will then emit vpcmpeqd even for -msse2 -mno-avx.
>
> There is really nothing wrong in what is on the trunk, no reason to patch
> anything. What the trunk does is, if you have AVX, you emit VEX encoded
> insn, if you don't have AVX, you emit normally encoded insn.
> And whether there is support to get cheap all ones is the business of
> standard_sse_constant_p function, which DTRT.
Probably, yes. But this just look pretty strange at first glance.
Something like this looks much more straightforward to me:
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 963cc3d..4aff647 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10859,7 +10859,7 @@ standard_sse_constant_opcode (rtx_insn *insn, rtx x)
|| get_attr_mode (insn) == MODE_V8DF
|| get_attr_mode (insn) == MODE_V16SF)
return "vpternlogd\t{$0xFF, %g0, %g0, %g0|%g0, %g0, %g0, 0xFF}";
- if (TARGET_AVX2 || get_attr_mode (insn) == 16)
+ if ((TARGET_AVX2 && get_attr_mode (insn) == 32) || TARGET_AVX)
return "vpcmpeqd\t%0, %0, %0";
else
return "pcmpeqd\t%0, %0";
But yes, if standard_sse_constant_p () is used properly there's no need for
the change.
--
Thanks, K
