Thanks. Committed to trunk with that fix: Author: kyukhin Date: Fri Nov 7 20:42:36 2014 New Revision: 217237
URL: https://gcc.gnu.org/viewcvs?rev=217237&root=gcc&view=rev Log: PR target/63534 gcc/ * config/i386/i386.md (builtin_setjmp_receiver): Use pic_offset_table_rtx for PIC register. (nonlocal_goto_receiver): Delete. Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/i386.md On Fri, Nov 7, 2014 at 11:33 PM, H.J. Lu <hjl.to...@gmail.com> wrote: > On Thu, Nov 6, 2014 at 5:01 AM, Evgeny Stupachenko <evstu...@gmail.com> wrote: >> Now I see that equiv reload could be special for PIC register. Let's >> apply more conservative patch. >> >> Darwin bootstrap passed with the patch applied on r216304 (along with >> already committed to trunk patches from PR63618 and PR63620). >> >> 2014-11-06 Evgeny Stupachenko <evstu...@gmail.com> >> >> PR target/63534 >> * config/i386/i386.c (builtin_setjmp_receiver): Use >> pic_offset_table_rtx >> for PIC register. >> (nonlocal_goto_receiver): Delete. > > It should be config/i386/i386.md, not config/i386/i386.c. > >> >> diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md >> index 7b1dd79..0df66ea 100644 >> --- a/gcc/config/i386/i386.md >> +++ b/gcc/config/i386/i386.md >> @@ -16989,10 +16989,9 @@ >> if (TARGET_MACHO) >> { >> rtx xops[3]; >> - rtx picreg = gen_rtx_REG (Pmode, PIC_OFFSET_TABLE_REGNUM); >> rtx_code_label *label_rtx = gen_label_rtx (); >> emit_insn (gen_set_got_labelled (pic_offset_table_rtx, label_rtx)); >> - xops[0] = xops[1] = picreg; >> + xops[0] = xops[1] = pic_offset_table_rtx; >> xops[2] = machopic_gen_offset (gen_rtx_LABEL_REF (SImode, label_rtx)); >> ix86_expand_binary_operator (MINUS, SImode, xops); >> } >> @@ -17002,36 +17001,6 @@ >> DONE; >> }) >> >> -(define_insn_and_split "nonlocal_goto_receiver" >> - [(unspec_volatile [(const_int 0)] UNSPECV_NLGR)] >> - "TARGET_MACHO && !TARGET_64BIT && flag_pic" >> - "#" >> - "&& reload_completed" >> - [(const_int 0)] >> -{ >> - if (crtl->uses_pic_offset_table) >> - { >> - rtx xops[3]; >> - rtx label_rtx = gen_label_rtx (); >> - rtx tmp; >> - >> - /* Get a new pic base. */ >> - emit_insn (gen_set_got_labelled (pic_offset_table_rtx, label_rtx)); >> - /* Correct this with the offset from the new to the old. */ >> - xops[0] = xops[1] = pic_offset_table_rtx; >> - label_rtx = gen_rtx_LABEL_REF (SImode, label_rtx); >> - tmp = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, label_rtx), >> - UNSPEC_MACHOPIC_OFFSET); >> - xops[2] = gen_rtx_CONST (Pmode, tmp); >> - ix86_expand_binary_operator (MINUS, SImode, xops); >> - } >> - else >> - /* No pic reg restore needed. */ >> - emit_note (NOTE_INSN_DELETED); >> - >> - DONE; >> -}) >> - >> ;; Avoid redundant prefixes by splitting HImode arithmetic to SImode. >> ;; Do not split instructions with mask registers. >> (define_split >> >> On Wed, Nov 5, 2014 at 3:59 PM, Evgeny Stupachenko <evstu...@gmail.com> >> wrote: >>> On Tue, Nov 4, 2014 at 1:40 AM, Jeff Law <l...@redhat.com> wrote: >>>> On 11/01/14 06:39, Evgeny Stupachenko wrote: >>>>> >>>>> When PIC register is pseudo there is nothing special about it's value >>>>> that setjmp can hurt. So if the pseudo register lives across >>>>> setjmp_receiver RA should care about correct allocation (in case it is >>>>> not saved/restored, it should go on stack). gcc.dg tests and specs >>>>> I've tested behave like this. >>>> >>>> If the allocator picked a call-clobbered register for the PIC register, >>>> then >>>> we're obviously OK since the setjmp has to be expected to clobber the PIC >>>> register. >>>> >>>> But if the PIC register is in a call-saved register, then it's going to be >>>> assumed to not be clobbered across calls and I don't believe that is >>>> guaranteed for builtin setjmp/longjmp. Those restore SP, FP and an ARGP, >>>> but not anything else by default. >>> >>> I still don't see what is special for PIC register here. PIC pseudo >>> now behave as every other pseudo register. >>> If we assume that setjmp can change a pseudo register value we need >>> IRA/LRA "magic" for each pseudo register. >>> >>> I believe that when we had EBX fixed, IRA/LRA don't save/restore it >>> anywhere. >>> Therefore we had to care about EBX value in special cases like >>> setjmp/non-local goto. >>> Now RA cares about PIC pseudo as well as about correct allocation for >>> any pseudo register. >>> >>>> >>>> So the callee might have clobbered the call saved hard register, expecting >>>> to restore its value in its epilogue. But due to the longjmp, that >>>> epilogue >>>> never gets called and thus the call-saved register won't have the right >>>> value in the receiver. >>>> >>>> Now if your argument is that IRA/LRA handle this, that's fine, a pointer to >>>> that code would be appreciated so that it can be quickly audited. Certainly >>>> the old local-alloc/global-alloc had magic for setjmp/longjmp and maybe >>>> IRA/LRA does too, but it's better to be sure than just assume. >>>> >>>>> >>>>> The initial problem comes from non-local goto as it tries to emit >>>>> pseudo PIC register after reload. >>>> >>>> ? You mean it emits a reference to the pseudo into RTL? That would >>>> indicate that the allocators never put the pseudo into a hard register?!? >>>> RTL dumps with a few pointers to key insns would help here. >>> >>> Correct, that is why Darwin crashes with ICE on non-local goto. >>> We still have: >>> >>> (define_insn_and_split "nonlocal_goto_receiver" >>> [(unspec_volatile [(const_int 0)] UNSPECV_NLGR)] >>> "TARGET_MACHO && !TARGET_64BIT && flag_pic" >>> "#" >>> "&& reload_completed" >>> [(const_int 0)] >>> { >>> if (crtl->uses_pic_offset_table) >>> { >>> rtx xops[3]; >>> rtx label_rtx = gen_label_rtx (); >>> rtx tmp; >>> >>> /* Get a new pic base. */ >>> emit_insn (gen_set_got_labelled (pic_offset_table_rtx, label_rtx)); >>> ..... >>> Here for MAC only we are trying to use pseudo PIC: >>> pic_offset_table_rtx when reload_completed. >>> >>>> >>>> jeff >>>> >>>> > > > > -- > H.J.
