Re: [PARCH 1/2, x86, PR63534] Fix darwin bootstrap

[Evgeny Stupachenko <evstupac at gmail dot com>]

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  <evstupac@gmail.com>

        PR target/63534
        * config/i386/i386.c (builtin_setjmp_receiver): Use pic_offset_table_rtx
        for PIC register.
        (nonlocal_goto_receiver): Delete.

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 <evstupac@gmail.com> wrote:
> On Tue, Nov 4, 2014 at 1:40 AM, Jeff Law <law@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
>>
>>