diff mbox

[i386,AVX] Fix typo in i386.c/standard_sse_constant_opcode ().

Message ID 20160323114753.GC59478@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Kirill Yukhin March 23, 2016, 11:47 a.m. UTC 
On 23 Mar 12:41, Jakub Jelinek wrote:
> On Wed, Mar 23, 2016 at 02:28:03PM +0300, Kirill Yukhin wrote:
> > Hello,
> > 
> > `vpcmpeqd' insn is only available in AVX2, however
> > [1] added check for AVX instead. Looks like a typo.
> > 
> > Patch in the bottom fixes that.
> > 
> > Bootstrapped, regtest still run.
> > Is it ok for main trunk if pass?
> 
> ??
> I see
> VEX.NDS.128.66.0F.WIG 76 /r	RVM	V/V	AVX	Compare packed doublewords in xmm3/m128 and
> VPCMPEQD xmm1, xmm2, xmm3/m128				xmm2 for equality.
> and
> VEX.NDS.256.66.0F.WIG 76 /r	RVM	V/V	AVX2	Compare packed doublewords in ymm3/m256 and
> VPCMPEQD ymm1, ymm2, ymm3 /m256				ymm2 for equality.
> in the ISA pdfs.
> 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.

--
Thanks, K

commit ae82089e3a4244d870737a7022e66e87042da811
Author: Kirill Yukhin <kirill.yukhin@intel.com>
Date:   Wed Mar 23 14:08:07 2016 +0300

    AVX2. Emit vpcmpeqd for const_m1 only if AVX2 is enabled.

Comments

Jakub Jelinek March 23, 2016, 11:55 a.m. UTC | #1 
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.

> commit ae82089e3a4244d870737a7022e66e87042da811
> Author: Kirill Yukhin <kirill.yukhin@intel.com>
> Date:   Wed Mar 23 14:08:07 2016 +0300
> 
>     AVX2. Emit vpcmpeqd for const_m1 only if AVX2 is enabled.
> 
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 1639704..963cc3d 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_AVX)
> +      if (TARGET_AVX2 || get_attr_mode (insn) == 16)
>         return "vpcmpeqd\t%0, %0, %0";
>        else
>         return "pcmpeqd\t%0, %0";

	Jakub
diff mbox

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 1639704..963cc3d 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_AVX)
+      if (TARGET_AVX2 || get_attr_mode (insn) == 16)
        return "vpcmpeqd\t%0, %0, %0";
       else
        return "pcmpeqd\t%0, %0";