This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] set -mabi=altivec with -ftree-vectorize


On Tue, Jul 31, 2007 at 09:20:42AM -0400, Daniel Jacobowitz wrote:
> With -mabi=altivec, v0 through v19 are call-used (call-clobbered,
> caller-saved).  Without -mabi=altivec, no AltiVec registers are saved
> on the stack (ref: first_altivec_reg_to_save).  Saving and restoring
> them would be a bit inefficient, due to the inadequate stack
> alignment.  But they're not mark call-used either, so GCC assumes it
> can use them.
> 
> In my opinion, if we are not going to use the AltiVec ABI, we should
> either not use the AltiVec registers or else save and restore them
> all.  Would it work to mark them all call-used for -maltivec, and
> then save them to the stack by allocating eight bytes extra and
> masking the stack pointer for sufficient alignment?  It wouldn't even
> be that inefficient.  Of course it would be slower than what we have
> today where we don't even bother to save them.  But I don't see how
> it's legitimate to use the current -maltivec in anything but a leaf
> function, which is undocumented and pretty bad.

I have been working on patches which add a .gnu_attribute entry
describing the vector ABI.  This is primarily for GDB's benefit, since
a number of tests in its testsuite fail on -mabi=no-altivec if AltiVec
hardware is detected.  But at the same time, I added support to the
linker to warn if you mix -mabi=no-altivec and -mabi=altivec code.
That caused a bunch of testcases in the GCC testsuite to fail.

Those testcases compile, link, and run code with -mabi=altivec.  This
is an ABI-changing option, so assuming it is compatible with the
system libraries is a little strange.  I tried just removing the
option, and hit the same bug that Zdenek and David are talking about
in this thread.  When one function using AltiVec calls another using
AltiVec, the second is likely to clobber registers that the first
expects to stay valid.

This patch improves the current state of things a bit.  In the
-maltivec -mabi=no-altivec case, we used to mark all AltiVec registers
as call-saved but not save them.  Now we mark them all call-used.
Obviously the code isn't great in non-leaf functions, but it should be
more correct.  And this version has no ABI implications.  I included
the new .gnu_attribute; this won't immediately add the warning,
but I'll post the binutils patch for that in a moment.

There's a potential improvement: define the non-AltiVec variants of
various ABIs to treat the upper registers as call-saved just like
the AltiVec variants do, and save and restore them in the prologue.
The unwind information for this is a little tricky to write and I
don't know how realistic tweaking those ABIs is in practice.

There's also a potential problem: see my message on gcc@ about the
stack boundary.  We assume a 16-byte stack boundary when using
-maltivec and indeed much of the code generated here is going to
misbehave if the stack is only 8-byte aligned.  Both local variables
and stack slots will be offset by eight bytes since lvx / stvx ignore
the low bits of the address.

I have tested this patch on the powerpc.exp and vmx.exp test cases,
on -m32, -m32 -msoft-float, and -m64.  Full test runs on those targets
are in progress now.  Is it OK, assuming they pass?

-- 
Daniel Jacobowitz
CodeSourcery

2007-08-07  Daniel Jacobowitz  <dan@codesourcery.com>

	* gcc.target/powerpc/altivec-consts.c: Remove -mabi=altivec.
	* gcc.target/powerpc/altivec-varargs-1.c: Likewise.
	* gcc.dg/vmx/vmx.exp: Likewise.

	* config/rs6000/rs6000.c (rs6000_file_start): Output a .gnu_attribute
	directive for the current vector ABI.
	(rs6000_conditional_register_usage): Mark call-saved AltiVec registers
	call-used if ! TARGET_ALTIVEC_ABI.
	* config/rs6000/rs6000.h (CALL_USED_REGISTERS): Mark the first 20
	AltiVec registers call-used.
	(CALL_REALLY_USED_REGISTERS): Likewise.

Index: gcc/testsuite/gcc.target/powerpc/altivec-consts.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/altivec-consts.c	(revision 127159)
+++ gcc/testsuite/gcc.target/powerpc/altivec-consts.c	(working copy)
@@ -1,6 +1,6 @@
 /* { dg-do run { target powerpc*-*-* } } */
 /* { dg-require-effective-target powerpc_altivec_ok } */
-/* { dg-options "-maltivec -mabi=altivec -O2" } */
+/* { dg-options "-maltivec -O2" } */
 
 /* Check that "easy" AltiVec constants are correctly synthesized.  */
 
Index: gcc/testsuite/gcc.target/powerpc/altivec-varargs-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/altivec-varargs-1.c	(revision 127159)
+++ gcc/testsuite/gcc.target/powerpc/altivec-varargs-1.c	(working copy)
@@ -1,6 +1,6 @@
 /* { dg-do run { target powerpc*-*-* } } */
 /* { dg-require-effective-target powerpc_altivec_ok } */
-/* { dg-options "-maltivec -mabi=altivec -fno-inline" } */
+/* { dg-options "-maltivec -fno-inline" } */
 
 #include <stdarg.h>
 #include <signal.h>
Index: gcc/testsuite/gcc.dg/vmx/vmx.exp
===================================================================
--- gcc/testsuite/gcc.dg/vmx/vmx.exp	(revision 127159)
+++ gcc/testsuite/gcc.dg/vmx/vmx.exp	(working copy)
@@ -31,7 +31,7 @@ if {![istarget powerpc*-*-*]
 # nothing but extensions.
 global DEFAULT_VMXCFLAGS
 if ![info exists DEFAULT_VMXCFLAGS] then {
-    set DEFAULT_VMXCFLAGS "-maltivec -mabi=altivec -std=gnu99"
+    set DEFAULT_VMXCFLAGS "-maltivec -std=gnu99"
 }
 
 # If the target system supports AltiVec instructions, the default action
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 127159)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -2316,8 +2316,14 @@ rs6000_file_start (void)
 
 #ifdef HAVE_AS_GNU_ATTRIBUTE
   if (TARGET_32BIT && DEFAULT_ABI == ABI_V4)
-    fprintf (file, "\t.gnu_attribute 4, %d\n",
-	     (TARGET_HARD_FLOAT && TARGET_FPRS) ? 1 : 2);
+    {
+      fprintf (file, "\t.gnu_attribute 4, %d\n",
+	       (TARGET_HARD_FLOAT && TARGET_FPRS) ? 1 : 2);
+      fprintf (file, "\t.gnu_attribute 8, %d\n",
+	       (TARGET_ALTIVEC_ABI ? 2
+		: TARGET_SPE_ABI ? 3
+		: 1));
+    }
 #endif
 
   if (DEFAULT_ABI == ABI_AIX || (TARGET_ELF && flag_pic == 2))
@@ -4141,8 +4147,13 @@ rs6000_conditional_register_usage (void)
       call_really_used_regs[VRSAVE_REGNO] = 1;
     }
 
-  if (TARGET_ALTIVEC_ABI)
-    for (i = FIRST_ALTIVEC_REGNO; i < FIRST_ALTIVEC_REGNO + 20; ++i)
+  /* If we are not using the AltiVec ABI, pretend that the normally
+     call-saved registers are also call-used.  We could use them
+     normally if we saved and restored them in the prologue; that
+     would require using the alignment padding around the register
+     save area, and some care with unwinding information.  */
+  if (! TARGET_ALTIVEC_ABI)
+    for (i = FIRST_ALTIVEC_REGNO + 20; i <= LAST_ALTIVEC_REGNO; ++i)
       call_used_regs[i] = call_really_used_regs[i] = 1;
 }
 
Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 127159)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -698,8 +698,8 @@ extern enum rs6000_nop_insertion rs6000_
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
    1, 1, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 1,	   \
    /* AltiVec registers.  */			   \
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
+   1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, \
+   1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
    1, 1						   \
    , 1, 1, 1                                       \
 }
@@ -717,8 +717,8 @@ extern enum rs6000_nop_insertion rs6000_
    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
    1, 1, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 1,	   \
    /* AltiVec registers.  */			   \
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
+   1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, \
+   1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
    0, 0						   \
    , 0, 0, 0                                       \
 }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]