diff mbox series

[rs6000] Fix ICE caused by recent patch: Generate lvx and stvx without swaps for aligned vector loads and stores

Message ID 6ddb7fcb-49be-efa2-63ac-4128300e73ac@linux.vnet.ibm.com
State New
Headers show
Series [rs6000] Fix ICE caused by recent patch: Generate lvx and stvx without swaps for aligned vector loads and stores | expand

Commit Message

Kelvin Nilsen Jan. 16, 2018, 5:15 p.m. UTC 
A patch committed on 2018-01-10 is causing an ICE with existing test
program $GCC_SRC/gcc/testsuite/gcc.target/powerpc/pr83399.c, when
compiled with the -m32 option.  At the time of the commit, it was
thought that this was a problem with the recent resolution of PR83399.
However, further investigation revealed a problem with the patch that
was just committed.  The generated code did not distinguish between 32-
and 64-bit targets.

This patch corrects that problem.

This has been bootstrapped and tested without regressions on
powerpc64le-unknown-linux (P8) and on powerpc64-unknown-linux (P7) with
both -m32 and -m64 target options.  Is this ok for trunk?


gcc/ChangeLog:

2018-01-16  Kelvin Nilsen  <kelvin@gcc.gnu.org>

	* config/rs6000/rs6000-p8swap.c (rs6000_gen_stvx): Generate
	different rtl trees depending on TARGET_64BIT.
	(rs6000_gen_lvx): Likewise.

Comments

Segher Boessenkool Jan. 16, 2018, 5:43 p.m. UTC | #1 
Hi Kelvin,

On Tue, Jan 16, 2018 at 11:15:12AM -0600, Kelvin Nilsen wrote:
> 
> A patch committed on 2018-01-10 is causing an ICE with existing test
> program $GCC_SRC/gcc/testsuite/gcc.target/powerpc/pr83399.c, when
> compiled with the -m32 option.  At the time of the commit, it was
> thought that this was a problem with the recent resolution of PR83399.
> However, further investigation revealed a problem with the patch that
> was just committed.  The generated code did not distinguish between 32-
> and 64-bit targets.
> 
> This patch corrects that problem.
> 
> This has been bootstrapped and tested without regressions on
> powerpc64le-unknown-linux (P8) and on powerpc64-unknown-linux (P7) with
> both -m32 and -m64 target options.  Is this ok for trunk?
> 
> 
> gcc/ChangeLog:
> 
> 2018-01-16  Kelvin Nilsen  <kelvin@gcc.gnu.org>
> 

	PR target/83399
?  Or is there another PR?

> 	* config/rs6000/rs6000-p8swap.c (rs6000_gen_stvx): Generate
> 	different rtl trees depending on TARGET_64BIT.
> 	(rs6000_gen_lvx): Likewise.
> 
> Index: gcc/config/rs6000/rs6000-p8swap.c
> ===================================================================
> --- gcc/config/rs6000/rs6000-p8swap.c	(revision 256710)
> +++ gcc/config/rs6000/rs6000-p8swap.c	(working copy)
> @@ -1554,23 +1554,31 @@ rs6000_gen_stvx (enum machine_mode mode, rtx dest_
>        op1 = XEXP (memory_address, 0);
>        op2 = XEXP (memory_address, 1);
>        if (mode == V16QImode)
> -	stvx = gen_altivec_stvx_v16qi_2op (src_exp, op1, op2);
> +	stvx = TARGET_64BIT ? gen_altivec_stvx_v16qi_2op (src_exp, op1, op2)
> +	  : gen_altivec_stvx_v16qi_2op_si (src_exp, op1, op2);

Please indent this like

	stvx = TARGET_64BIT
	       ? gen_altivec_stvx_v16qi_2op (src_exp, op1, op2)
	       : gen_altivec_stvx_v16qi_2op_si (src_exp, op1, op2);

>        if (mode == V16QImode)
> -	stvx = gen_altivec_stvx_v16qi_1op (src_exp, memory_address);
> +	stvx = TARGET_64BIT ?
> +	  gen_altivec_stvx_v16qi_1op (src_exp, memory_address)
> +	  : gen_altivec_stvx_v16qi_1op_si (src_exp, memory_address);

You should never have ? at the end of line; and ? and : indent with the
controlling expression.  So:

	stvx = TARGET_64BIT
	       ? gen_altivec_stvx_v16qi_1op (src_exp, memory_address)
	       : gen_altivec_stvx_v16qi_1op_si (src_exp, memory_address);

Similar everywhere.  Okay with that changed.  Thanks!


Segher
diff mbox series

Patch

Index: gcc/config/rs6000/rs6000-p8swap.c
===================================================================
--- gcc/config/rs6000/rs6000-p8swap.c	(revision 256710)
+++ gcc/config/rs6000/rs6000-p8swap.c	(working copy)
@@ -1554,23 +1554,31 @@  rs6000_gen_stvx (enum machine_mode mode, rtx dest_
       op1 = XEXP (memory_address, 0);
       op2 = XEXP (memory_address, 1);
       if (mode == V16QImode)
-	stvx = gen_altivec_stvx_v16qi_2op (src_exp, op1, op2);
+	stvx = TARGET_64BIT ? gen_altivec_stvx_v16qi_2op (src_exp, op1, op2)
+	  : gen_altivec_stvx_v16qi_2op_si (src_exp, op1, op2);
       else if (mode == V8HImode)
-	stvx = gen_altivec_stvx_v8hi_2op (src_exp, op1, op2);
+	stvx = TARGET_64BIT ? gen_altivec_stvx_v8hi_2op (src_exp, op1, op2)
+	  : gen_altivec_stvx_v8hi_2op_si (src_exp, op1, op2);
 #ifdef HAVE_V8HFmode
       else if (mode == V8HFmode)
-	stvx = gen_altivec_stvx_v8hf_2op (src_exp, op1, op2);
+	stvx = TARGET_64BIT ? gen_altivec_stvx_v8hf_2op (src_exp, op1, op2)
+	  : gen_altivec_stvx_v8hf_2op_si (src_exp, op1, op2);
 #endif
       else if (mode == V4SImode)
-	stvx = gen_altivec_stvx_v4si_2op (src_exp, op1, op2);
+	stvx = TARGET_64BIT ? gen_altivec_stvx_v4si_2op (src_exp, op1, op2)
+	  : gen_altivec_stvx_v4si_2op_si (src_exp, op1, op2);
       else if (mode == V4SFmode)
-	stvx = gen_altivec_stvx_v4sf_2op (src_exp, op1, op2);
+	stvx = TARGET_64BIT ? gen_altivec_stvx_v4sf_2op (src_exp, op1, op2)
+	  : gen_altivec_stvx_v4sf_2op_si (src_exp, op1, op2);
       else if (mode == V2DImode)
-	stvx = gen_altivec_stvx_v2di_2op (src_exp, op1, op2);
+	stvx = TARGET_64BIT ? gen_altivec_stvx_v2di_2op (src_exp, op1, op2)
+	  : gen_altivec_stvx_v2di_2op_si (src_exp, op1, op2);
       else if (mode == V2DFmode)
-	stvx = gen_altivec_stvx_v2df_2op (src_exp, op1, op2);
+	stvx = TARGET_64BIT ? gen_altivec_stvx_v2df_2op (src_exp, op1, op2)
+	  : gen_altivec_stvx_v2df_2op_si (src_exp, op1, op2);
       else if (mode == V1TImode)
-	stvx = gen_altivec_stvx_v1ti_2op (src_exp, op1, op2);
+	stvx = TARGET_64BIT ? gen_altivec_stvx_v1ti_2op (src_exp, op1, op2)
+	  : gen_altivec_stvx_v1ti_2op_si (src_exp, op1, op2);
       else
 	/* KFmode, TFmode, other modes not expected in this context.  */
 	gcc_unreachable ();
@@ -1578,23 +1586,39 @@  rs6000_gen_stvx (enum machine_mode mode, rtx dest_
   else				/* REG_P (memory_address) */
     {
       if (mode == V16QImode)
-	stvx = gen_altivec_stvx_v16qi_1op (src_exp, memory_address);
+	stvx = TARGET_64BIT ?
+	  gen_altivec_stvx_v16qi_1op (src_exp, memory_address)
+	  : gen_altivec_stvx_v16qi_1op_si (src_exp, memory_address);
       else if (mode == V8HImode)
-	stvx = gen_altivec_stvx_v8hi_1op (src_exp, memory_address);
+	stvx = TARGET_64BIT ?
+	  gen_altivec_stvx_v8hi_1op (src_exp, memory_address)
+	  : gen_altivec_stvx_v8hi_1op_si (src_exp, memory_address);
 #ifdef HAVE_V8HFmode
       else if (mode == V8HFmode)
-	stvx = gen_altivec_stvx_v8hf_1op (src_exp, memory_address);
+	stvx = TARGET_64BIT ?
+	  gen_altivec_stvx_v8hf_1op (src_exp, memory_address)
+	  : gen_altivec_stvx_v8hf_1op_si (src_exp, memory_address);
 #endif
       else if (mode == V4SImode)
-	stvx = gen_altivec_stvx_v4si_1op (src_exp, memory_address);
+	stvx =TARGET_64BIT ?
+	  gen_altivec_stvx_v4si_1op (src_exp, memory_address)
+	  : gen_altivec_stvx_v4si_1op_si (src_exp, memory_address);
       else if (mode == V4SFmode)
-	stvx = gen_altivec_stvx_v4sf_1op (src_exp, memory_address);
+	stvx = TARGET_64BIT ?
+	  gen_altivec_stvx_v4sf_1op (src_exp, memory_address)
+	  : gen_altivec_stvx_v4sf_1op_si (src_exp, memory_address);
       else if (mode == V2DImode)
-	stvx = gen_altivec_stvx_v2di_1op (src_exp, memory_address);
+	stvx = TARGET_64BIT ?
+	  gen_altivec_stvx_v2di_1op (src_exp, memory_address)
+	  : gen_altivec_stvx_v2di_1op_si (src_exp, memory_address);
       else if (mode == V2DFmode)
-	stvx = gen_altivec_stvx_v2df_1op (src_exp, memory_address);
+	stvx = TARGET_64BIT ?
+	  gen_altivec_stvx_v2df_1op (src_exp, memory_address)
+	  : gen_altivec_stvx_v2df_1op_si (src_exp, memory_address);
       else if (mode == V1TImode)
-	stvx = gen_altivec_stvx_v1ti_1op (src_exp, memory_address);
+	stvx = TARGET_64BIT ?
+	  gen_altivec_stvx_v1ti_1op (src_exp, memory_address)
+	  : gen_altivec_stvx_v1ti_1op_si (src_exp, memory_address);
       else
 	/* KFmode, TFmode, other modes not expected in this context.  */
 	gcc_unreachable ();
@@ -1702,23 +1726,31 @@  rs6000_gen_lvx (enum machine_mode mode, rtx dest_e
       op2 = XEXP (memory_address, 1);
 
       if (mode == V16QImode)
-	lvx = gen_altivec_lvx_v16qi_2op (dest_exp, op1, op2);
+	lvx = TARGET_64BIT ? gen_altivec_lvx_v16qi_2op (dest_exp, op1, op2)
+	  : gen_altivec_lvx_v16qi_2op_si (dest_exp, op1, op2);
       else if (mode == V8HImode)
-	lvx = gen_altivec_lvx_v8hi_2op (dest_exp, op1, op2);
+	lvx = TARGET_64BIT ? gen_altivec_lvx_v8hi_2op (dest_exp, op1, op2)
+	  : gen_altivec_lvx_v8hi_2op_si (dest_exp, op1, op2);
 #ifdef HAVE_V8HFmode
       else if (mode == V8HFmode)
-	lvx = gen_altivec_lvx_v8hf_2op (dest_exp, op1, op2);
+	lvx = TARGET_64BIT ? gen_altivec_lvx_v8hf_2op (dest_exp, op1, op2)
+	  : gen_altivec_lvx_v8hf_2op_si (dest_exp, op1, op2);
 #endif
       else if (mode == V4SImode)
-	lvx = gen_altivec_lvx_v4si_2op (dest_exp, op1, op2);
+	lvx = TARGET_64BIT ? gen_altivec_lvx_v4si_2op (dest_exp, op1, op2)
+	  : gen_altivec_lvx_v4si_2op_si (dest_exp, op1, op2);
       else if (mode == V4SFmode)
-	lvx = gen_altivec_lvx_v4sf_2op (dest_exp, op1, op2);
+	lvx = TARGET_64BIT ? gen_altivec_lvx_v4sf_2op (dest_exp, op1, op2)
+	  : gen_altivec_lvx_v4sf_2op_si (dest_exp, op1, op2);
       else if (mode == V2DImode)
-	lvx = gen_altivec_lvx_v2di_2op (dest_exp, op1, op2);
+	lvx = TARGET_64BIT ? gen_altivec_lvx_v2di_2op (dest_exp, op1, op2)
+	  : gen_altivec_lvx_v2di_2op_si (dest_exp, op1, op2);
       else if (mode == V2DFmode)
-	lvx = gen_altivec_lvx_v2df_2op (dest_exp, op1, op2);
+	lvx = TARGET_64BIT ? gen_altivec_lvx_v2df_2op (dest_exp, op1, op2)
+	  : gen_altivec_lvx_v2df_2op_si (dest_exp, op1, op2);
       else if (mode == V1TImode)
-	lvx = gen_altivec_lvx_v1ti_2op (dest_exp, op1, op2);
+	lvx = TARGET_64BIT ? gen_altivec_lvx_v1ti_2op (dest_exp, op1, op2)
+	  : gen_altivec_lvx_v1ti_2op_si (dest_exp, op1, op2);
       else
 	/* KFmode, TFmode, other modes not expected in this context.  */
 	gcc_unreachable ();
@@ -1726,23 +1758,39 @@  rs6000_gen_lvx (enum machine_mode mode, rtx dest_e
   else				/* REG_P (memory_address) */
     {
       if (mode == V16QImode)
-	lvx = gen_altivec_lvx_v16qi_1op (dest_exp, memory_address);
+	lvx = TARGET_64BIT ?
+	  gen_altivec_lvx_v16qi_1op (dest_exp, memory_address)
+	  : gen_altivec_lvx_v16qi_1op_si (dest_exp, memory_address);
       else if (mode == V8HImode)
-	lvx = gen_altivec_lvx_v8hi_1op (dest_exp, memory_address);
+	lvx = TARGET_64BIT ?
+	  gen_altivec_lvx_v8hi_1op (dest_exp, memory_address)
+	  : gen_altivec_lvx_v8hi_1op_si (dest_exp, memory_address);
 #ifdef HAVE_V8HFmode
       else if (mode == V8HFmode)
-	lvx = gen_altivec_lvx_v8hf_1op (dest_exp, memory_address);
+	lvx = TARGET_64BIT ?
+	  gen_altivec_lvx_v8hf_1op (dest_exp, memory_address)
+	  : gen_altivec_lvx_v8hf_1op_si (dest_exp, memory_address);
 #endif
       else if (mode == V4SImode)
-	lvx = gen_altivec_lvx_v4si_1op (dest_exp, memory_address);
+	lvx = TARGET_64BIT ?
+	  gen_altivec_lvx_v4si_1op (dest_exp, memory_address)
+	  : gen_altivec_lvx_v4si_1op_si (dest_exp, memory_address);
       else if (mode == V4SFmode)
-	lvx = gen_altivec_lvx_v4sf_1op (dest_exp, memory_address);
+	lvx = TARGET_64BIT ?
+	  gen_altivec_lvx_v4sf_1op (dest_exp, memory_address)
+	  : gen_altivec_lvx_v4sf_1op_si (dest_exp, memory_address);
       else if (mode == V2DImode)
-	lvx = gen_altivec_lvx_v2di_1op (dest_exp, memory_address);
+	lvx = TARGET_64BIT ?
+	  gen_altivec_lvx_v2di_1op (dest_exp, memory_address)
+	  : gen_altivec_lvx_v2di_1op_si (dest_exp, memory_address);
       else if (mode == V2DFmode)
-	lvx = gen_altivec_lvx_v2df_1op (dest_exp, memory_address);
+	lvx = TARGET_64BIT ?
+	  gen_altivec_lvx_v2df_1op (dest_exp, memory_address)
+	  : gen_altivec_lvx_v2df_1op_si (dest_exp, memory_address);
       else if (mode == V1TImode)
-	lvx = gen_altivec_lvx_v1ti_1op (dest_exp, memory_address);
+	lvx = TARGET_64BIT ?
+	  gen_altivec_lvx_v1ti_1op (dest_exp, memory_address)
+	  : gen_altivec_lvx_v1ti_1op_si (dest_exp, memory_address);
       else
 	/* KFmode, TFmode, other modes not expected in this context.  */
 	gcc_unreachable ();