On 4 Dec 2011, at 06:35, Mike Stump wrote:

On Nov 30, 2011, at 6:28 AM, Iain Sandoe wrote:
While trying to track down the vector unwind problems on ppc- darwin, I made some tidy-ups for "save_world()". In the end, that was not where the main problem, lay - but I did find a few things wrong there on the way - they should be fixed, even if there's no specific bug filed at present.

I'm also attaching a second patch which is purely cosmetic white- space/comment tidies I'd also like to apply.

checked on powerpc-darwin{8-G4,9-G5} and crosses to powerpc-eabisim and powerpc-ibm-aix6.1.3.0

-  /* On Darwin, the unwind routines are compiled without
-     TARGET_ALTIVEC, and use save_world to save/restore the
-     altivec registers when necessary.  */
+ /* When generating code for earlier versions of Darwin, which might run on + hardware with or without Altivec, we use out-of-line save/ restores in + function prologues/epilogues that require it. These routines determine
+     whether to save/restore Altivec at runtime.  */

So, we need to first settle on how the library is compiled before this becomes true...

The patch does not change any functionality here.
This was a ( clearly failed ;-) ) attempt to make the comment more useful to newcomers to the code ...
(withdrawn, can think again sometime later).

+save_world:
+       trap
+
+       .private_extern eh_rest_world_r10
+eh_rest_world_r10:
+       trap

So, I can't see the necessity of doing this. I think it is bad style to generate code that will die at runtime. I'd rather have it not link.

done - there would now be a link fail if it were specified.

(although I would point out, FTR, that the code would have always crashed if it had ever been invoked - since it was only m32 capable).

[[FWIW, I made a working m64 version, which we could resurrect should there be some reason we decided that out-of-line world saves were useful everywhere]]

OK for trunk?
Iain

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c  (revision 181990)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -19899,7 +19899,9 @@ rs6000_emit_prologue (void)
      used in this function, and do the corresponding magic in the
      epilogue.  */
 
-  if (TARGET_ALTIVEC && TARGET_ALTIVEC_VRSAVE
+  if (!WORLD_SAVE_P (info)
+      && TARGET_ALTIVEC
+      && TARGET_ALTIVEC_VRSAVE
       && info->vrsave_mask != 0)
     {
       rtx reg, mem, vrsave;
@@ -19915,15 +19917,12 @@ rs6000_emit_prologue (void)
       else
         emit_insn (gen_rtx_SET (VOIDmode, reg, vrsave));
 
-      if (!WORLD_SAVE_P (info))
-        {
-          /* Save VRSAVE.  */
-          offset = info->vrsave_save_offset + sp_offset;
-          mem = gen_frame_mem (SImode,
-                               gen_rtx_PLUS (Pmode, frame_reg_rtx,
-                                             GEN_INT (offset)));
-          insn = emit_move_insn (mem, reg);
-        }
+      /* Save VRSAVE.  */
+      offset = info->vrsave_save_offset + sp_offset;
+      mem = gen_frame_mem (SImode,
+                          gen_rtx_PLUS (Pmode, frame_reg_rtx, 
+                                        GEN_INT (offset)));
+      insn = emit_move_insn (mem, reg);
 
       /* Include the registers in the mask.  */
       emit_insn (gen_iorsi3 (reg, reg, GEN_INT ((int) info->vrsave_mask)));
Index: libgcc/config/rs6000/darwin-world.S
===================================================================
--- libgcc/config/rs6000/darwin-world.S (revision 181990)
+++ libgcc/config/rs6000/darwin-world.S (working copy)
@@ -24,6 +24,8 @@
  * <http://www.gnu.org/licenses/>.
  */ 
 
+#ifndef __ppc64__
+
        .machine ppc7400
 .data
        .align 2
@@ -33,12 +35,7 @@
 .non_lazy_symbol_pointer
 L_has_vec$non_lazy_ptr:
        .indirect_symbol __cpu_has_altivec
-#ifdef __ppc64__
-       .quad   0
-#else
        .long   0
-#endif
-
 #else
 
 /* For static, "pretend" we have a non-lazy-pointer.  */
@@ -57,12 +54,11 @@ L_has_vec$non_lazy_ptr:
    provided by the System Framework to determine this.)
 
    SAVE_WORLD takes R0 (the caller`s caller`s return address) and R11
-   (the stack frame size) as parameters.  It returns VRsave in R0 if
-   we`re on a CPU with vector regs.
+   (the stack frame size) as parameters.  It returns the updated VRsave
+   in R0 if we`re on a CPU with vector regs.
 
-   With gcc3, we now need to save and restore CR as well, since gcc3's
-   scheduled prologs can cause comparisons to be moved before calls to
-   save_world!
+   For gcc3 onward, we need to save and restore CR as well, since scheduled
+   prologs can cause comparisons to be moved before calls to save_world.
 
    USES: R0 R11 R12  */
 
@@ -143,69 +139,62 @@ L$saveVMX:
        stvx v30,r11,r12
        mfspr r0,VRsave
        li r11,-16
-       stvx v31,r11,r12
-                               /* VRsave lives at -224(R1)  */
-       stw r0,0(r12)
+       stvx v31,r11,r12        
+       stw r0,0(r12)           /* VRsave lives at -224(R1).  */
+       ori r0,r0,0xfff         /* We just saved these.  */
+       mtspr VRsave,r0
        blr
 
+/* rest_world  is jumped to, not called, so no need to worry about LR.
+   clobbers R0, R7, R11 and R12.  This just undoes the work done above.  */
 
+       .private_extern rest_world
+rest_world:
+                               
+       lwz r11, 0(r1)          /* Pickup previous SP  */
+       li r7, 0                /* Stack offset is zero, r10 is ignored.  */
+       b Lrest_world_eh_r7r8
+
 /* eh_rest_world_r10 is jumped to, not called, so no need to worry about LR.
    R10 is the C++ EH stack adjust parameter, we return to the caller`s caller.
 
-   USES: R0 R10 R11 R12   and R7 R8
-   RETURNS: C++ EH Data registers (R3 - R6.)
+   clobbers: R0, R7, R11 and R12
+   uses    : R10
+   RETURNS : C++ EH Data registers (R3 - R6).  */
 
-   We now set up R7/R8 and jump to rest_world_eh_r7r8.
-
-   rest_world doesn't use the R10 stack adjust parameter, nor does it
-   pick up the R3-R6 exception handling stuff.  */
-
-.private_extern rest_world
-rest_world:
-                               /* Pickup previous SP  */
-       lwz r11, 0(r1)
-       li r7, 0
-       lwz r8, 8(r11)
-       li r10, 0
-       b rest_world_eh_r7r8
-
-.private_extern eh_rest_world_r10
+       .private_extern eh_rest_world_r10
 eh_rest_world_r10:
-                               /* Pickup previous SP  */
-       lwz r11, 0(r1)
-       mr  r7,r10
-       lwz r8, 8(r11)
-                       /* pickup the C++ EH data regs (R3 - R6.)  */
+                               
+       lwz r11, 0(r1)          /* Pickup previous SP  */
+       mr  r7,r10              /* Stack offset.  */
+       
+       /* pickup the C++ EH data regs (R3 - R6.)  */
        lwz r6,-420(r11)
        lwz r5,-424(r11)
        lwz r4,-428(r11)
        lwz r3,-432(r11)
 
-       b rest_world_eh_r7r8
+       /* Fall through to Lrest_world_eh_r7r8.  */
 
-/* rest_world_eh_r7r8 is jumped to -- not called! -- when we're doing
-   the exception-handling epilog.  R7 contains the offset to add to
-   the SP, and R8 contains the 'real' return address.
+/* When we are doing the exception-handling epilog, R7 contains the offset to
+   add to the SP.
 
-   USES: R0 R11 R12  [R7/R8]
-   RETURNS: C++ EH Data registers (R3 - R6.)  */
+   clobbers: R0, R11 and R12
+   uses    : R7.  */
 
-rest_world_eh_r7r8:
+Lrest_world_eh_r7r8:
+       /* See if we have Altivec.  */
        bcl 20,31,Lr7r8$pb
 Lr7r8$pb: mflr r12
-       lwz r11,0(r1)
-                               /* R11 := previous SP  */
+
        addis r12,r12,ha16(L_has_vec$non_lazy_ptr-Lr7r8$pb)
        lwz r12,lo16(L_has_vec$non_lazy_ptr-Lr7r8$pb)(r12)
-       lwz r0,4(r11)
-                               /* R0 := old CR  */
-       lwz r12,0(r12)
-                               /* R12 := HAS_VEC  */
-       mtcr r0 
+       lwz r12,0(r12)          /* R12 := HAS_VEC  */
        cmpwi r12,0
        lmw r13,-220(r11)
        beq L.rest_world_fp_eh
-                               /* restore VRsave and V20..V31  */
+
+       /* We have Altivec, restore VRsave and V20..V31  */
        lwz r0,-224(r11)
        li r12,-416
        mtspr VRsave,r0
@@ -234,6 +223,7 @@ Lr7r8$pb: mflr r12
        lvx v31,r11,r12
 
 L.rest_world_fp_eh:
+       lwz r0,4(r11)           /* recover saved CR  */
        lfd f14,-144(r11)
        lfd f15,-136(r11)
        lfd f16,-128(r11)
@@ -251,9 +241,12 @@ L.rest_world_fp_eh:
        lfd f28,-32(r11)
        lfd f29,-24(r11)
        lfd f30,-16(r11)
-                       /* R8 is the exception-handler's address  */
-       mtctr r8
-       lfd f31,-8(r11)
-                       /* set SP to original value + R7 offset  */
-       add r1,r11,r7
+       mtcr r0                 /* restore the saved cr.  */
+       lwz r0, 8(r11)          /* Pick up the 'real' return address.  */
+       lfd f31,-8(r11)         
+       mtctr r0                /* exception-handler ret. address  */
+       add r1,r11,r7           /* set SP to original value + R7 offset  */
        bctr
+#endif
+/* we should never be called on ppc64 for this ... */
+/* Done.  */

Reply via email to