On Tue, Feb 04, 2014 at 12:21:14PM +0000, Richard Sandiford wrote:
> Jakub Jelinek <ja...@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).
Then just handle REG_CFA_ADJUST_CFA note in
insn_stack_adjust_offset_pre_post. Handling REG_CFA_DEF_CFA
note there would be harder (as you need to know the current cfa at that
point), so perhaps just punt there if you see REG_CFA_DEF_CFA note.
Jakub
