This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: var-tracking and s390's LM(G)
- From: Richard Sandiford <rsandifo at linux dot vnet dot ibm dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org, Andreas dot Krebbel at de dot ibm dot com, uweigand at de dot ibm dot com
- Date: Wed, 05 Feb 2014 15:32:22 +0000
- Subject: Re: var-tracking and s390's LM(G)
- Authentication-results: sourceware.org; auth=none
- References: <874n4ff92i dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <20140204110456 dot GF12671 at tucnak dot redhat dot com> <87ha8fdsd6 dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <20140204114438 dot GG12671 at tucnak dot redhat dot com> <87k3dbcblx dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <20140204142949 dot GR12671 at tucnak dot redhat dot com>
Jakub Jelinek <jakub@redhat.com> writes:
> On Tue, Feb 04, 2014 at 12:21:14PM +0000, Richard Sandiford wrote:
>> Jakub Jelinek <jakub@redhat.com> writes:
>> >> But then we wouldn't be able to use var-tracking when __builtin_eh_return
>> >> is used, since in that case replacing the (set (reg ...) (mem ...))
>> >> with a (plus ...) would be incorrect -- the value we're loading from the
>> >> stack will have had a variable adjustment applied. And I know from painful
>> >> experience that being able to debug the unwind code is very useful. :-)
>> >
>> > Aren't functions using EH_RETURN typically using frame pointer?
>>
>> Sorry, forgot to respond to this bit. On s390, _Unwind_RaiseException and
>> _Unwind_ForcedUnwind don't use a frame pointer, at least not on trunk.
>> %r11 is used as a general scratch register instead. E.g.:
>>
>> 00002ba8 <_Unwind_ForcedUnwind>:
>> 2ba8: 90 6f f0 18 stm %r6,%r15,24(%r15)
>> 2bac: 0d d0 basr %r13,%r0
>> 2bae: 60 40 f0 50 std %f4,80(%r15)
>> 2bb2: 60 60 f0 58 std %f6,88(%r15)
>> 2bb6: a7 fa fd f8 ahi %r15,-520
>> 2bba: 58 c0 d0 9e l %r12,158(%r13)
>> 2bbe: 58 10 d0 9a l %r1,154(%r13)
>> 2bc2: 18 b2 lr %r11,%r2
>> ...
>> 2c10: 98 6f f2 20 lm %r6,%r15,544(%r15)
>> 2c14: 07 f4 br %r4
>
> Looking at pr54693-2.c -O2 -g -m64 -march=z196 (was that the one you meant)
> with your patches, I see:
> (insn/f:TI 122 30 31 4 (parallel [
> (set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 80 [0x50])) [3 S8 A8])
> (reg:DI 10 %r10))
> (set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 88 [0x58])) [3 S8 A8])
> (reg:DI 11 %r11))
> (set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 96 [0x60])) [3 S8 A8])
> (reg:DI 12 %r12))
> (set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 104 [0x68])) [3 S8 A8])
> (reg:DI 13 %r13))
> (set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 112 [0x70])) [3 S8 A8])
> (reg:DI 14 %r14))
> (set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 120 [0x78])) [3 S8 A8])
> (reg/f:DI 15 %r15))
> ]) pr54693-2.c:16 94 {*store_multiple_di}
> (expr_list:REG_DEAD (reg:DI 14 %r14)
> (expr_list:REG_DEAD (reg:DI 13 %r13)
> (expr_list:REG_DEAD (reg:DI 12 %r12)
> (expr_list:REG_DEAD (reg:DI 11 %r11)
> (expr_list:REG_DEAD (reg:DI 10 %r10)
> (nil)))))))
> ...
> (insn/f 123 31 124 4 (set (reg/f:DI 15 %r15)
> (plus:DI (reg/f:DI 15 %r15)
> (const_int -160 [0xffffffffffffff60]))) pr54693-2.c:16 65 {*la_64}
> (expr_list:REG_FRAME_RELATED_EXPR (set (reg/f:DI 15 %r15)
> (plus:DI (reg/f:DI 15 %r15)
> (const_int -160 [0xffffffffffffff60])))
> (nil)))
> ...
> (insn/f:TI 135 134 136 7 (parallel [
> (set (reg:DI 10 %r10)
> (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 240 [0xf0])) [3 S8 A8]))
> (set (reg:DI 11 %r11)
> (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 248 [0xf8])) [3 S8 A8]))
> (set (reg:DI 12 %r12)
> (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 256 [0x100])) [3 S8 A8]))
> (set (reg:DI 13 %r13)
> (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 264 [0x108])) [3 S8 A8]))
> (set (reg:DI 14 %r14)
> (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 272 [0x110])) [3 S8 A8]))
> (set (reg/f:DI 15 %r15)
> (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 280 [0x118])) [3 S8 A8]))
> ]) pr54693-2.c:25 92 {*load_multiple_di}
> (expr_list:REG_CFA_DEF_CFA (plus:DI (reg/f:DI 15 %r15)
> (const_int 160 [0xa0]))
> (expr_list:REG_CFA_RESTORE (reg/f:DI 15 %r15)
> (expr_list:REG_CFA_RESTORE (reg:DI 14 %r14)
> (expr_list:REG_CFA_RESTORE (reg:DI 13 %r13)
> (expr_list:REG_CFA_RESTORE (reg:DI 12 %r12)
> (expr_list:REG_CFA_RESTORE (reg:DI 11 %r11)
> (expr_list:REG_CFA_RESTORE (reg:DI 10 %r10)
> (nil)))))))))
> In a function that doesn't need frame pointer, I'd say this is a serious
> bloat of the unwind info, you are saving/restoring %r15 not because you have
> to, but just that it seems to be cheaper for the target. So, I'd say you
> shouldn't emit DW_CFA_offset 15, 5 on the insn 122 in the prologue and
> DW_CFA_restore 15 in the epilogue (twice in this case), that just waste
> of .eh_frame/.debug_frame space. I'd say you should represent in
> this case the *store_multiple_di as REG_FRAME_RELATED_EXPR
> with all but the last set in the PARALLEL, and on the *load_multiple_di
> remove REG_CFA_RESTORE (r15) note and change REG_CFA_DEF_CFA note to
> REG_CFA_ADJUST_CFA note which would say that stack pointer has been
> incremented by 160 bytes (epilogue expansion knows that value).
I think at the moment we're relying on the "DW_CFA_offset 15"s to
provide correct %r15 values on eh_return. Every non-leaf function
saves the stack pointer and the unwind routines track the %r15 save
slots like they would track any call-saved register. In other words,
the final eh_return %r15 comes directly from a save slot, without any
CFA-specific handling. If some frames omit the CFI for the %r15 save
then we'll just keep the save slot from the previous (inner) frame instead.
So if we remove the %r15 saves from the CFI then I think we'd have to use
the EH_RETURN_STACKADJ_RTX mechanism instead. This would cope with frames
that save %r15 as well as those that don't, thanks to the final line of:
#ifdef EH_RETURN_STACKADJ_RTX
/* Special handling here: Many machines do not use a frame pointer,
and track the CFA only through offsets from the stack pointer from
one frame to the next. In this case, the stack pointer is never
stored, so it has no saved address in the context. What we do
have is the CFA from the previous stack frame.
In very special situations (such as unwind info for signal return),
there may be location expressions that use the stack pointer as well.
Do this conditionally for one frame. This allows the unwind info
for one frame to save a copy of the stack pointer from the previous
frame, and be able to use much easier CFA mechanisms to do it.
Always zap the saved stack pointer value for the next frame; carrying
the value over from one frame to another doesn't make sense. */
_Unwind_SpTmp tmp_sp;
if (!_Unwind_GetGRPtr (&orig_context, __builtin_dwarf_sp_column ()))
_Unwind_SetSpColumn (&orig_context, context->cfa, &tmp_sp);
_Unwind_SetGRPtr (context, __builtin_dwarf_sp_column (), NULL);
#endif
The new unwind routines would handle 4.9+ and pre-4.9 frames, but pre-4.9
unwind routines wouldn't handle 4.9+ frames, so would that require a version
bump on the unwind symbols?
There should be no problem with the epilogue bits though.
Thanks,
Richard