From patchwork Sun Dec 4 20:23:51 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Iain Sandoe X-Patchwork-Id: 129191 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 97B511007D5 for ; Mon, 5 Dec 2011 07:24:14 +1100 (EST) Received: (qmail 9129 invoked by alias); 4 Dec 2011 20:24:12 -0000 Received: (qmail 9111 invoked by uid 22791); 4 Dec 2011 20:24:10 -0000 X-SWARE-Spam-Status: No, hits=-0.4 required=5.0 tests=AWL, BAYES_50, RCVD_IN_DNSWL_NONE, TW_LV, TW_LW, TW_TV X-Spam-Check-By: sourceware.org Received: from c2bthomr14.btconnect.com (HELO mail.btconnect.com) (213.123.20.132) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 04 Dec 2011 20:23:55 +0000 Received: from host81-138-1-83.in-addr.btopenworld.com (EHLO thor.office) ([81.138.1.83]) by c2bthomr14.btconnect.com with ESMTP id FKF95187; Sun, 04 Dec 2011 20:23:53 +0000 (GMT) Cc: GCC Patches , David Edelsohn Message-Id: <2A206914-6A78-4F47-A7D0-F06BCC7CFD4A@sandoe-acoustics.co.uk> From: Iain Sandoe To: Mike Stump In-Reply-To: <709069DD-7ABC-4212-9E37-08AE9BBEA7B7@comcast.net> Mime-Version: 1.0 (Apple Message framework v936) Subject: Re: [Patch PPC/Darwin] some tidy-ups for save_world (and a prelude to splitting it out of the rs6000 code). Date: Sun, 4 Dec 2011 20:23:51 +0000 References: <709069DD-7ABC-4212-9E37-08AE9BBEA7B7@comcast.net> X-Mirapoint-IP-Reputation: reputation=Good-1, source=Queried, refid=tid=0001.0A0B0303.4EDBD6D7.0040, actions=tag X-Junkmail-Premium-Raw: score=8/50, refid=2.7.2:2011.12.4.194515:17:8.129, ip=81.138.1.83, rules=__MULTIPLE_RCPTS_CC_X2, __HAS_MSGID, __SANE_MSGID, __MSGID_APPLEMAIL, __TO_MALFORMED_2, __CT, __CTYPE_HAS_BOUNDARY, __CTYPE_MULTIPART, CTYPE_MULTIPART_NO_QUOTE, __CTYPE_MULTIPART_MIXED, __MIME_VERSION, __MIME_VERSION_APPLEMAIL, __BOUNCE_CHALLENGE_SUBJ, __BOUNCE_NDR_SUBJ_EXEMPT, __HAS_X_MAILER, __X_MAILER_APPLEMAIL, TXT_ATTACHED, __ANY_URI, __URI_NO_MAILTO, __CP_URI_IN_BODY, BODYTEXTP_SIZE_3000_LESS, __MIME_TEXT_ONLY, RDNS_GENERIC_POOLED, RDNS_SUSP_GENERIC, __USER_AGENT_APPLEMAIL, RDNS_SUSP, MIME_TEXT_ONLY_MP_MIXED, MULTIPLE_RCPTS X-Junkmail-Signature-Raw: score=unknown, refid=str=0001.0A0B0201.4EDBD6D9.0044, ss=1, re=0.000, fgs=0, ip=0.0.0.0, so=2011-07-25 19:15:43, dmn=2011-05-27 18:58:46, mode=multiengine X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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 @@ * . */ +#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. */