diff mbox

[i386] Fix alignment check for AVX-512 masked store

Message ID 20151202135154.GA4176@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich Dec. 2, 2015, 1:51 p.m. UTC
Hi,

This patch fixes wrong alignment check in <avx512>_store<mode>_mask
pattern.  Currently we check a register operand instead of a memory
one.  This fixes segfault on 481.wrf compiled at -O3 for KNL target.
I bootstrapped and tested this patch on x86_64-unknown-linux-gnu.

I got a bunch of new failures:

FAIL: gcc.target/i386/avx512vl-vmovapd-1.c scan-assembler-times vmovapd[ \\t]+[^{\n]*%xmm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
FAIL: gcc.target/i386/avx512vl-vmovapd-1.c scan-assembler-times vmovapd[ \\t]+[^{\n]*%xmm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
FAIL: gcc.target/i386/avx512vl-vmovapd-1.c scan-assembler-times vmovapd[ \\t]+[^{\n]*%ymm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
FAIL: gcc.target/i386/avx512vl-vmovapd-1.c scan-assembler-times vmovapd[ \\t]+[^{\n]*%ymm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
FAIL: gcc.target/i386/avx512vl-vmovaps-1.c scan-assembler-times vmovaps[ \\t]+[^{\n]*%xmm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
FAIL: gcc.target/i386/avx512vl-vmovaps-1.c scan-assembler-times vmovaps[ \\t]+[^{\n]*%xmm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
FAIL: gcc.target/i386/avx512vl-vmovaps-1.c scan-assembler-times vmovaps[ \\t]+[^{\n]*%ymm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
FAIL: gcc.target/i386/avx512vl-vmovaps-1.c scan-assembler-times vmovaps[ \\t]+[^{\n]*%ymm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1

With patch applied test generates vmovup[sd] because memory
references don't have proper alignment set.  Since this is
another bug and it's actually a performance one, I think
this patch should go to trunk.

Thanks,
Ilya
--
gcc/

2015-12-02  Ilya Enkovich  <enkovich.gnu@gmail.com>

	* config/i386/sse.md (<avx512>_store<mode>_mask): Fix
	operand checked for alignment.

Comments

Kirill Yukhin Dec. 4, 2015, 1:45 p.m. UTC | #1
Hi Ilya,
On 02 Dec 16:51, Ilya Enkovich wrote:
> Hi,
> 
> This patch fixes wrong alignment check in <avx512>_store<mode>_mask
> pattern.  Currently we check a register operand instead of a memory
> one.  This fixes segfault on 481.wrf compiled at -O3 for KNL target.
> I bootstrapped and tested this patch on x86_64-unknown-linux-gnu.
> 
> I got a bunch of new failures:
> 
> FAIL: gcc.target/i386/avx512vl-vmovapd-1.c scan-assembler-times vmovapd[ \\t]+[^{\n]*%xmm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
> FAIL: gcc.target/i386/avx512vl-vmovapd-1.c scan-assembler-times vmovapd[ \\t]+[^{\n]*%xmm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
> FAIL: gcc.target/i386/avx512vl-vmovapd-1.c scan-assembler-times vmovapd[ \\t]+[^{\n]*%ymm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
> FAIL: gcc.target/i386/avx512vl-vmovapd-1.c scan-assembler-times vmovapd[ \\t]+[^{\n]*%ymm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
> FAIL: gcc.target/i386/avx512vl-vmovaps-1.c scan-assembler-times vmovaps[ \\t]+[^{\n]*%xmm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
> FAIL: gcc.target/i386/avx512vl-vmovaps-1.c scan-assembler-times vmovaps[ \\t]+[^{\n]*%xmm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
> FAIL: gcc.target/i386/avx512vl-vmovaps-1.c scan-assembler-times vmovaps[ \\t]+[^{\n]*%ymm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
> FAIL: gcc.target/i386/avx512vl-vmovaps-1.c scan-assembler-times vmovaps[ \\t]+[^{\n]*%ymm[0-9]+[^\n]*\\){%k[1-7]}(?:\n|[ \\t]+#) 1
> 
> With patch applied test generates vmovup[sd] because memory
> references don't have proper alignment set.  Since this is
> another bug and it's actually a performance one, I think
> this patch should go to trunk.

Your patch definetely fixes stability issue.
New fails are performance issues.

OK for main trunk. Pls, file regression bug in Bugzilla against new fails.

--
Thanks, K


> 
> Thanks,
> Ilya
> --
> gcc/
> 
> 2015-12-02  Ilya Enkovich  <enkovich.gnu@gmail.com>
> 
> 	* config/i386/sse.md (<avx512>_store<mode>_mask): Fix
> 	operand checked for alignment.
> 
> 
> diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
> index e7b517a..d65ed0c 100644
> --- a/gcc/config/i386/sse.md
> +++ b/gcc/config/i386/sse.md
> @@ -1051,7 +1051,7 @@
>        sse_suffix = "<ssescalarsize>";
>      }
>  
> -  if (misaligned_operand (operands[1], <MODE>mode))
> +  if (misaligned_operand (operands[0], <MODE>mode))
>      align = "u";
>    else
>      align = "a";
diff mbox

Patch

diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index e7b517a..d65ed0c 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -1051,7 +1051,7 @@ 
       sse_suffix = "<ssescalarsize>";
     }
 
-  if (misaligned_operand (operands[1], <MODE>mode))
+  if (misaligned_operand (operands[0], <MODE>mode))
     align = "u";
   else
     align = "a";