[PATCH, powerpc], Fix PR48857, pass/return V2DI as other vector types

[PATCH, powerpc], Fix PR48857, pass/return V2DI as other vector types

[Michael Meissner]

[-- Attachment #1: Type: text/plain, Size: 2057 bytes --]

When I added VSX support to the powerpc, I overlooked passing and return
V2DImode arguments, since the only machine operation that supports V2DI is
vector floating point conversion.  Consequentally, V2DI types were passed and
returned in GPRs instead of the vector registers on power7.

This patch fixes that so that V2DImode values are passed and returned like
other vector types.

I did a bootstrap and make check with no regressions, comparing it to a build
without the patch.  I also wrote a program that passed and returned every
single type, and I compared the assembly ouptut.  With the exception of
functions that return or are passed V2DI arguments, the code is identical.  I
tested:

	-m64			(implies -mabi=altivec)
	-m32 -mabi=altivec
	-m32 -mabi=no-altivec	(no difference here)

Is this patch ok to install?  I will also want to install it in the 4.6 and
possibly 4.5 trees as well.

[gcc]
2011-05-03  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/48857
	* config/rs6000/rs6000.h (VSX_SCALAR_MODE): Delete.
	(VSX_MODE): Ditto.
	(VSX_MOVE_MODE): Ditto.
	(ALTIVEC_OR_VSX_VECTOR_MODE): New macro, combine all Altivec and
	VSX vector types.  Add V2DImode.
	(HARD_REGNO_CALLER_SAVE_MODE): Use it instead of
	ALTIVEC_VECTOR_MODE and VSX_VECTOR_MODE calls.
	(MODES_TIEABLE_P): Ditto.

	* config/rs6000/rs6000.c (rs6000_emit_move): Use
	ALTIVEC_OR_VSX_MODE instead of ALTIVEC_VECTOR_MODE and
	VSX_VECTOR_MODE.
	(init_cumulative_args): Ditto.
	(rs6000_function_arg_boundary): Ditto.
	(rs6000_function_arg_advance_1): Ditto.
	(rs6000_function_arg): Ditto.
	(rs6000_function_ok_for_sibcall): Ditto.
	(emit_frame_save): Ditto.
	(rs6000_function_value): Ditto.
	(rs6000_libcall_value): Ditto.

[gcc/testsuite]
2011-05-03  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/48857
	* gcc.target/powerpc/pr48857.c: New file, make sure V2DI arguments
	are passed and returned in vector registers.

-- 
Michael Meissner, IBM
5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
meissner@linux.vnet.ibm.com	fax +1 (978) 399-6899

[-- Attachment #2: gcc-power7.patch240 --]
[-- Type: text/plain, Size: 8945 bytes --]

[gcc]
2011-05-03  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/48857
	* config/rs6000/rs6000.h (VSX_SCALAR_MODE): Delete.
	(VSX_MODE): Ditto.
	(VSX_MOVE_MODE): Ditto.
	(ALTIVEC_OR_VSX_VECTOR_MODE): New macro, combine all Altivec and
	VSX vector types.  Add V2DImode.
	(HARD_REGNO_CALLER_SAVE_MODE): Use it instead of
	ALTIVEC_VECTOR_MODE and VSX_VECTOR_MODE calls.
	(MODES_TIEABLE_P): Ditto.

	* config/rs6000/rs6000.c (rs6000_emit_move): Use
	ALTIVEC_OR_VSX_MODE instead of ALTIVEC_VECTOR_MODE and
	VSX_VECTOR_MODE.
	(init_cumulative_args): Ditto.
	(rs6000_function_arg_boundary): Ditto.
	(rs6000_function_arg_advance_1): Ditto.
	(rs6000_function_arg): Ditto.
	(rs6000_function_ok_for_sibcall): Ditto.
	(emit_frame_save): Ditto.
	(rs6000_function_value): Ditto.
	(rs6000_libcall_value): Ditto.

[gcc/testsuite]
2011-05-03  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/48857
	* gcc.target/powerpc/pr48857.c: New file, make sure V2DI arguments
	are passed and returned in vector registers.


Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 173266)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -1007,10 +1007,9 @@ extern unsigned rs6000_pointer_size;
 
 /* When setting up caller-save slots (MODE == VOIDmode) ensure we allocate
    enough space to account for vectors in FP regs. */
-#define HARD_REGNO_CALLER_SAVE_MODE(REGNO, NREGS, MODE)	\
-  (TARGET_VSX						\
-   && ((MODE) == VOIDmode || VSX_VECTOR_MODE (MODE)	\
-       || ALTIVEC_VECTOR_MODE (MODE))			\
+#define HARD_REGNO_CALLER_SAVE_MODE(REGNO, NREGS, MODE)			\
+  (TARGET_VSX								\
+   && ((MODE) == VOIDmode || ALTIVEC_OR_VSX_VECTOR_MODE (MODE))		\
    && FP_REGNO_P (REGNO)				\
    ? V2DFmode						\
    : choose_hard_reg_mode ((REGNO), (NREGS), false))
@@ -1026,25 +1025,16 @@ extern unsigned rs6000_pointer_size;
 	 ((MODE) == V4SFmode		\
 	  || (MODE) == V2DFmode)	\
 
-#define VSX_SCALAR_MODE(MODE)		\
-	((MODE) == DFmode)
-
-#define VSX_MODE(MODE)			\
-	(VSX_VECTOR_MODE (MODE)		\
-	 || VSX_SCALAR_MODE (MODE))
-
-#define VSX_MOVE_MODE(MODE)		\
-	(VSX_VECTOR_MODE (MODE)		\
-	 || VSX_SCALAR_MODE (MODE)	\
-	 || ALTIVEC_VECTOR_MODE (MODE)	\
-	 || (MODE) == TImode)
-
 #define ALTIVEC_VECTOR_MODE(MODE)	\
 	 ((MODE) == V16QImode		\
 	  || (MODE) == V8HImode		\
 	  || (MODE) == V4SFmode		\
 	  || (MODE) == V4SImode)
 
+#define ALTIVEC_OR_VSX_VECTOR_MODE(MODE)				\
+  (ALTIVEC_VECTOR_MODE (MODE) || VSX_VECTOR_MODE (MODE)			\
+   || (MODE) == V2DImode)
+
 #define SPE_VECTOR_MODE(MODE)		\
 	((MODE) == V4HImode          	\
          || (MODE) == V2SFmode          \
@@ -1080,10 +1070,10 @@ extern unsigned rs6000_pointer_size;
    ? ALTIVEC_VECTOR_MODE (MODE2)		\
    : ALTIVEC_VECTOR_MODE (MODE2)		\
    ? ALTIVEC_VECTOR_MODE (MODE1)		\
-   : VSX_VECTOR_MODE (MODE1)			\
-   ? VSX_VECTOR_MODE (MODE2)			\
-   : VSX_VECTOR_MODE (MODE2)			\
-   ? VSX_VECTOR_MODE (MODE1)			\
+   : ALTIVEC_OR_VSX_VECTOR_MODE (MODE1)		\
+   ? ALTIVEC_OR_VSX_VECTOR_MODE (MODE2)		\
+   : ALTIVEC_OR_VSX_VECTOR_MODE (MODE2)		\
+   ? ALTIVEC_OR_VSX_VECTOR_MODE (MODE1)		\
    : 1)
 
 /* Post-reload, we can't use any new AltiVec registers, as we already
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 173266)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -7902,7 +7902,7 @@ rs6000_emit_move (rtx dest, rtx source, 
 
 /* Nonzero if we can use an AltiVec register to pass this arg.  */
 #define USE_ALTIVEC_FOR_ARG_P(CUM,MODE,TYPE,NAMED)		\
-  ((ALTIVEC_VECTOR_MODE (MODE) || VSX_VECTOR_MODE (MODE))	\
+  (ALTIVEC_OR_VSX_VECTOR_MODE (MODE)				\
    && (CUM)->vregno <= ALTIVEC_ARG_MAX_REG			\
    && TARGET_ALTIVEC_ABI					\
    && (NAMED))
@@ -8102,8 +8102,7 @@ init_cumulative_args (CUMULATIVE_ARGS *c
 	    }
 	  if (SCALAR_FLOAT_MODE_P (return_mode))
 	    rs6000_passes_float = true;
-	  else if (ALTIVEC_VECTOR_MODE (return_mode)
-		   || VSX_VECTOR_MODE (return_mode)
+	  else if (ALTIVEC_OR_VSX_VECTOR_MODE (return_mode)
 		   || SPE_VECTOR_MODE (return_mode))
 	    rs6000_passes_vector = true;
 	}
@@ -8218,7 +8217,7 @@ rs6000_function_arg_boundary (enum machi
 	       && int_size_in_bytes (type) >= 8
 	       && int_size_in_bytes (type) < 16))
     return 64;
-  else if ((ALTIVEC_VECTOR_MODE (mode) || VSX_VECTOR_MODE (mode))
+  else if (ALTIVEC_OR_VSX_VECTOR_MODE (mode)
 	   || (type && TREE_CODE (type) == VECTOR_TYPE
 	       && int_size_in_bytes (type) >= 16))
     return 128;
@@ -8438,7 +8437,7 @@ rs6000_function_arg_advance_1 (CUMULATIV
     {
       if (SCALAR_FLOAT_MODE_P (mode))
 	rs6000_passes_float = true;
-      else if (named && (ALTIVEC_VECTOR_MODE (mode) || VSX_VECTOR_MODE (mode)))
+      else if (named && ALTIVEC_OR_VSX_VECTOR_MODE (mode))
 	rs6000_passes_vector = true;
       else if (SPE_VECTOR_MODE (mode)
 	       && !cum->stdarg
@@ -8448,8 +8447,7 @@ rs6000_function_arg_advance_1 (CUMULATIV
 #endif
 
   if (TARGET_ALTIVEC_ABI
-      && (ALTIVEC_VECTOR_MODE (mode)
-	  || VSX_VECTOR_MODE (mode)
+      && (ALTIVEC_OR_VSX_VECTOR_MODE (mode)
 	  || (type && TREE_CODE (type) == VECTOR_TYPE
 	      && int_size_in_bytes (type) == 16)))
     {
@@ -9067,8 +9065,7 @@ rs6000_function_arg (CUMULATIVE_ARGS *cu
     else
       return gen_rtx_REG (mode, cum->vregno);
   else if (TARGET_ALTIVEC_ABI
-	   && (ALTIVEC_VECTOR_MODE (mode)
-	       || VSX_VECTOR_MODE (mode)
+	   && (ALTIVEC_OR_VSX_VECTOR_MODE (mode)
 	       || (type && TREE_CODE (type) == VECTOR_TYPE
 		   && int_size_in_bytes (type) == 16)))
     {
@@ -19355,14 +19352,12 @@ rs6000_function_ok_for_sibcall (tree dec
 	 here.  */
       FOREACH_FUNCTION_ARGS(fntype, type, args_iter)
 	if (TREE_CODE (type) == VECTOR_TYPE
-	    && (ALTIVEC_VECTOR_MODE (TYPE_MODE (type))
-		|| VSX_VECTOR_MODE (TYPE_MODE (type))))
+	    && ALTIVEC_OR_VSX_VECTOR_MODE (TYPE_MODE (type)))
 	  nvreg++;
 
       FOREACH_FUNCTION_ARGS(TREE_TYPE (current_function_decl), type, args_iter)
 	if (TREE_CODE (type) == VECTOR_TYPE
-	    && (ALTIVEC_VECTOR_MODE (TYPE_MODE (type))
-		|| VSX_VECTOR_MODE (TYPE_MODE (type))))
+	    && ALTIVEC_OR_VSX_VECTOR_MODE (TYPE_MODE (type)))
 	  nvreg--;
 
       if (nvreg > 0)
@@ -20075,7 +20070,7 @@ emit_frame_save (rtx frame_reg, rtx fram
 
   /* Some cases that need register indexed addressing.  */
   if ((TARGET_ALTIVEC_ABI && ALTIVEC_VECTOR_MODE (mode))
-      || (TARGET_VSX && VSX_VECTOR_MODE (mode))
+      || (TARGET_VSX && ALTIVEC_OR_VSX_VECTOR_MODE (mode))
       || (TARGET_E500_DOUBLE && mode == DFmode)
       || (TARGET_SPE_ABI
 	  && SPE_VECTOR_MODE (mode)
@@ -27361,11 +27356,7 @@ rs6000_function_value (const_tree valtyp
     return rs6000_complex_function_value (mode);
   else if (TREE_CODE (valtype) == VECTOR_TYPE
 	   && TARGET_ALTIVEC && TARGET_ALTIVEC_ABI
-	   && ALTIVEC_VECTOR_MODE (mode))
-    regno = ALTIVEC_ARG_RETURN;
-  else if (TREE_CODE (valtype) == VECTOR_TYPE
-	   && TARGET_VSX && TARGET_ALTIVEC_ABI
-	   && VSX_VECTOR_MODE (mode))
+	   && ALTIVEC_OR_VSX_VECTOR_MODE (mode))
     regno = ALTIVEC_ARG_RETURN;
   else if (TARGET_E500_DOUBLE && TARGET_HARD_FLOAT
 	   && (mode == DFmode || mode == DCmode
@@ -27405,12 +27396,9 @@ rs6000_libcall_value (enum machine_mode 
 	   && TARGET_HARD_FLOAT && TARGET_FPRS
            && ((TARGET_SINGLE_FLOAT && mode == SFmode) || TARGET_DOUBLE_FLOAT))
     regno = FP_ARG_RETURN;
-  else if (ALTIVEC_VECTOR_MODE (mode)
+  else if (ALTIVEC_OR_VSX_VECTOR_MODE (mode)
 	   && TARGET_ALTIVEC && TARGET_ALTIVEC_ABI)
     regno = ALTIVEC_ARG_RETURN;
-  else if (VSX_VECTOR_MODE (mode)
-	   && TARGET_VSX && TARGET_ALTIVEC_ABI)
-    regno = ALTIVEC_ARG_RETURN;
   else if (COMPLEX_MODE_P (mode) && targetm.calls.split_complex_arg)
     return rs6000_complex_function_value (mode);
   else if (TARGET_E500_DOUBLE && TARGET_HARD_FLOAT
Index: gcc/testsuite/gcc.target/powerpc/pr48857.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr48857.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr48857.c	(revision 0)
@@ -0,0 +1,25 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+/* { dg-options "-O2 -mcpu=power7 -mabi=altivec" } */
+/* { dg-final { scan-assembler-times "lxvd2x" 1 } } */
+/* { dg-final { scan-assembler-times "stxvd2x" 1 } } */
+/* { dg-final { scan-assembler-not "ld" } } */
+/* { dg-final { scan-assembler-not "lwz" } } */
+/* { dg-final { scan-assembler-not "stw" } } */
+/* { dg-final { scan-assembler-not "addi" } } */
+
+typedef vector long long v2di_type;
+
+v2di_type
+return_v2di (v2di_type *ptr)
+{
+  return *ptr;		/* should generate lxvd2x 34,0,3.  */
+}
+
+void
+pass_v2di (v2di_type arg, v2di_type *ptr)
+{
+  *ptr = arg;		/* should generate stxvd2x 34,0,{3,5}.  */
+}
+

Re: [PATCH, powerpc], Fix PR48857, pass/return V2DI as other vector types

[David Edelsohn]

On Tue, May 3, 2011 at 5:27 PM, Michael Meissner
<meissner@linux.vnet.ibm.com> wrote:
> When I added VSX support to the powerpc, I overlooked passing and return
> V2DImode arguments, since the only machine operation that supports V2DI is
> vector floating point conversion.  Consequentally, V2DI types were passed and
> returned in GPRs instead of the vector registers on power7.
>
> This patch fixes that so that V2DImode values are passed and returned like
> other vector types.
>
> I did a bootstrap and make check with no regressions, comparing it to a build
> without the patch.  I also wrote a program that passed and returned every
> single type, and I compared the assembly ouptut.  With the exception of
> functions that return or are passed V2DI arguments, the code is identical.  I
> tested:
>
>        -m64                    (implies -mabi=altivec)
>        -m32 -mabi=altivec
>        -m32 -mabi=no-altivec   (no difference here)
>
> Is this patch ok to install?  I will also want to install it in the 4.6 and
> possibly 4.5 trees as well.
>
> [gcc]
> 2011-05-03  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>        PR target/48857
>        * config/rs6000/rs6000.h (VSX_SCALAR_MODE): Delete.
>        (VSX_MODE): Ditto.
>        (VSX_MOVE_MODE): Ditto.
>        (ALTIVEC_OR_VSX_VECTOR_MODE): New macro, combine all Altivec and
>        VSX vector types.  Add V2DImode.
>        (HARD_REGNO_CALLER_SAVE_MODE): Use it instead of
>        ALTIVEC_VECTOR_MODE and VSX_VECTOR_MODE calls.
>        (MODES_TIEABLE_P): Ditto.
>
>        * config/rs6000/rs6000.c (rs6000_emit_move): Use
>        ALTIVEC_OR_VSX_MODE instead of ALTIVEC_VECTOR_MODE and
>        VSX_VECTOR_MODE.
>        (init_cumulative_args): Ditto.
>        (rs6000_function_arg_boundary): Ditto.
>        (rs6000_function_arg_advance_1): Ditto.
>        (rs6000_function_arg): Ditto.
>        (rs6000_function_ok_for_sibcall): Ditto.
>        (emit_frame_save): Ditto.
>        (rs6000_function_value): Ditto.
>        (rs6000_libcall_value): Ditto.
>
> [gcc/testsuite]
> 2011-05-03  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>        PR target/48857
>        * gcc.target/powerpc/pr48857.c: New file, make sure V2DI arguments
>        are passed and returned in vector registers.

What does this do to the ABI?  Haven't we now broken the ABI and
broken backwards compatibility?

- David

Re: [PATCH, powerpc], Fix PR48857, pass/return V2DI as other vector types

[Richard Guenther]

On Wed, May 4, 2011 at 4:51 AM, David Edelsohn <dje.gcc@gmail.com> wrote:
> On Tue, May 3, 2011 at 5:27 PM, Michael Meissner
> <meissner@linux.vnet.ibm.com> wrote:
>> When I added VSX support to the powerpc, I overlooked passing and return
>> V2DImode arguments, since the only machine operation that supports V2DI is
>> vector floating point conversion.  Consequentally, V2DI types were passed and
>> returned in GPRs instead of the vector registers on power7.
>>
>> This patch fixes that so that V2DImode values are passed and returned like
>> other vector types.
>>
>> I did a bootstrap and make check with no regressions, comparing it to a build
>> without the patch.  I also wrote a program that passed and returned every
>> single type, and I compared the assembly ouptut.  With the exception of
>> functions that return or are passed V2DI arguments, the code is identical.  I
>> tested:
>>
>>        -m64                    (implies -mabi=altivec)
>>        -m32 -mabi=altivec
>>        -m32 -mabi=no-altivec   (no difference here)
>>
>> Is this patch ok to install?  I will also want to install it in the 4.6 and
>> possibly 4.5 trees as well.
>>
>> [gcc]
>> 2011-05-03  Michael Meissner  <meissner@linux.vnet.ibm.com>
>>
>>        PR target/48857
>>        * config/rs6000/rs6000.h (VSX_SCALAR_MODE): Delete.
>>        (VSX_MODE): Ditto.
>>        (VSX_MOVE_MODE): Ditto.
>>        (ALTIVEC_OR_VSX_VECTOR_MODE): New macro, combine all Altivec and
>>        VSX vector types.  Add V2DImode.
>>        (HARD_REGNO_CALLER_SAVE_MODE): Use it instead of
>>        ALTIVEC_VECTOR_MODE and VSX_VECTOR_MODE calls.
>>        (MODES_TIEABLE_P): Ditto.
>>
>>        * config/rs6000/rs6000.c (rs6000_emit_move): Use
>>        ALTIVEC_OR_VSX_MODE instead of ALTIVEC_VECTOR_MODE and
>>        VSX_VECTOR_MODE.
>>        (init_cumulative_args): Ditto.
>>        (rs6000_function_arg_boundary): Ditto.
>>        (rs6000_function_arg_advance_1): Ditto.
>>        (rs6000_function_arg): Ditto.
>>        (rs6000_function_ok_for_sibcall): Ditto.
>>        (emit_frame_save): Ditto.
>>        (rs6000_function_value): Ditto.
>>        (rs6000_libcall_value): Ditto.
>>
>> [gcc/testsuite]
>> 2011-05-03  Michael Meissner  <meissner@linux.vnet.ibm.com>
>>
>>        PR target/48857
>>        * gcc.target/powerpc/pr48857.c: New file, make sure V2DI arguments
>>        are passed and returned in vector registers.
>
> What does this do to the ABI?  Haven't we now broken the ABI and
> broken backwards compatibility?

It at least looks like so.  You need to add appropriate changes.html
entries to all branches you apply this patch to.  I suppose the new
version matches what XLC does?

Richard.

> - David
>

Re: [PATCH, powerpc], Fix PR48857, pass/return V2DI as other vector types

[Peter Bergner]

On Wed, 2011-05-04 at 11:33 +0200, Richard Guenther wrote:
> On Wed, May 4, 2011 at 4:51 AM, David Edelsohn <dje.gcc@gmail.com> wrote:
> > On Tue, May 3, 2011 at 5:27 PM, Michael Meissner <meissner@linux.vnet.ibm.com> wrote:
> >>
> >> This patch fixes that so that V2DImode values are passed and returned like
> >> other vector types.
[snip]
> > What does this do to the ABI?  Haven't we now broken the ABI and
> > broken backwards compatibility?
> 
> It at least looks like so.  You need to add appropriate changes.html
> entries to all branches you apply this patch to.  I suppose the new
> version matches what XLC does?

Yes, XLC passes V2DI parameters and return values in Altivec registers
as per the ABI, so we're the buggy ones.

Peter

Re: [PATCH, powerpc], Fix PR48857, pass/return V2DI as other vector types

[Michael Meissner]

On Tue, May 03, 2011 at 10:51:51PM -0400, David Edelsohn wrote:
> What does this do to the ABI?  Haven't we now broken the ABI and
> broken backwards compatibility?

In this case, GCC wasn't following the ABI.  My patch will have GCC follow the
ABI.  The XL compiler also follows the ABI.  So we have to decide whether to
keep the current calling sequence which violates the ABI, or change the
compiler to follow the ABI, and break backwards compatiblity.

Given the machine doesn't have native VD2Imode operations, except for
conversion to/from floating point, and the bitwise operations, I suspect not to
many people use the data type.  Note, V2DImode was added in GCC 4.5 when the
VSX support was added.  It was not in the earlier Altivec support.

The MODES_TIEABLE_P part of the patch will need to go in to fix PR 48495 (it
was in looking at the bug we discovered that the compiler was not following the
ABI).

-- 
Michael Meissner, IBM
5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
meissner@linux.vnet.ibm.com	fax +1 (978) 399-6899

Re: [PATCH, powerpc], Fix PR48857, pass/return V2DI as other vector types

[Michael Meissner]

On Wed, May 04, 2011 at 11:33:43AM +0200, Richard Guenther wrote:
> On Wed, May 4, 2011 at 4:51 AM, David Edelsohn <dje.gcc@gmail.com> wrote:
> > What does this do to the ABI?  Haven't we now broken the ABI and
> > broken backwards compatibility?
> 
> It at least looks like so.  You need to add appropriate changes.html
> entries to all branches you apply this patch to.  I suppose the new
> version matches what XLC does?

Yes.  I will do that if the patch is approved.

As I said in my other reply, the patch will make us compatible with XLC.

-- 
Michael Meissner, IBM
5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
meissner@linux.vnet.ibm.com	fax +1 (978) 399-6899

Re: [PATCH, powerpc], Fix PR48857, pass/return V2DI as other vector types

[David Edelsohn]

On Tue, May 3, 2011 at 5:27 PM, Michael Meissner
<meissner@linux.vnet.ibm.com> wrote:
> When I added VSX support to the powerpc, I overlooked passing and return
> V2DImode arguments, since the only machine operation that supports V2DI is
> vector floating point conversion.  Consequentally, V2DI types were passed and
> returned in GPRs instead of the vector registers on power7.
>
> This patch fixes that so that V2DImode values are passed and returned like
> other vector types.
>
> I did a bootstrap and make check with no regressions, comparing it to a build
> without the patch.  I also wrote a program that passed and returned every
> single type, and I compared the assembly ouptut.  With the exception of
> functions that return or are passed V2DI arguments, the code is identical.  I
> tested:
>
>        -m64                    (implies -mabi=altivec)
>        -m32 -mabi=altivec
>        -m32 -mabi=no-altivec   (no difference here)
>
> Is this patch ok to install?  I will also want to install it in the 4.6 and
> possibly 4.5 trees as well.
>
> [gcc]
> 2011-05-03  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>        PR target/48857
>        * config/rs6000/rs6000.h (VSX_SCALAR_MODE): Delete.
>        (VSX_MODE): Ditto.
>        (VSX_MOVE_MODE): Ditto.
>        (ALTIVEC_OR_VSX_VECTOR_MODE): New macro, combine all Altivec and
>        VSX vector types.  Add V2DImode.
>        (HARD_REGNO_CALLER_SAVE_MODE): Use it instead of
>        ALTIVEC_VECTOR_MODE and VSX_VECTOR_MODE calls.
>        (MODES_TIEABLE_P): Ditto.
>
>        * config/rs6000/rs6000.c (rs6000_emit_move): Use
>        ALTIVEC_OR_VSX_MODE instead of ALTIVEC_VECTOR_MODE and
>        VSX_VECTOR_MODE.
>        (init_cumulative_args): Ditto.
>        (rs6000_function_arg_boundary): Ditto.
>        (rs6000_function_arg_advance_1): Ditto.
>        (rs6000_function_arg): Ditto.
>        (rs6000_function_ok_for_sibcall): Ditto.
>        (emit_frame_save): Ditto.
>        (rs6000_function_value): Ditto.
>        (rs6000_libcall_value): Ditto.
>
> [gcc/testsuite]
> 2011-05-03  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>        PR target/48857
>        * gcc.target/powerpc/pr48857.c: New file, make sure V2DI arguments
>        are passed and returned in vector registers.

The patch is okay, but please add a comment explaining why the
function_value and libcall_value changes are correct.

Thanks, David

Re: [PATCH, powerpc], Fix PR48857, pass/return V2DI as other vector types

[Uros Bizjak]

Hello!

> In this case, GCC wasn't following the ABI.  My patch will have GCC follow the
> ABI.  The XL compiler also follows the ABI.  So we have to decide whether to
> keep the current calling sequence which violates the ABI, or change the
> compiler to follow the ABI, and break backwards compatiblity.
>
> Given the machine doesn't have native VD2Imode operations, except for
> conversion to/from floating point, and the bitwise operations, I suspect not to
> many people use the data type.  Note, V2DImode was added in GCC 4.5 when the
> VSX support was added.  It was not in the earlier Altivec support.

IMO, the compiler should emit a note in this case, something like
[unfortunately, many...] examples in i386.c:

			  static bool warned;

			  if (!warned && warn_psabi)
			    {
			      warned = true;
			      inform (input_location,
				      "the ABI of passing struct with"
				      " a flexible array member has"
				      " changed in GCC 4.4");
			    }
Uros.