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 <rdsandiford at googlemail dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Tue, 04 Feb 2014 12:05:39 +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>
Jakub Jelinek <jakub@redhat.com> writes:
> On Tue, Feb 04, 2014 at 11:33:57AM +0000, Richard Sandiford wrote:
>> > So, how does the lmg insn look like in RTL dump on some problematic
>> > testcase?
>> > insn_stack_adjust_offset_pre_post already uses REG_FRAME_RELATED_EXPR,
>> > which is also a kind of CFI note (the oldest one), so likely the issue
>> > is just that it hasn't been adjusted to handle other newer REG_CFA_* notes
>> > that tell how the stack pointer is adjusted.
>>
>> It's just a (mem ...) access:
>>
>> (parallel
>> [...
>> (set (reg %r14) (mem:[SD]I (plus (reg ...) (const_int X1))))
>> (set (reg %r15) (mem:[SD]I (plus (reg ...) (const_int X2))))])
>
> I meant what reg notes it has (and why it doesn't use
> REG_FRAME_RELATED_EXPR).
It has a REG_CFA_RESTORE for each loaded register. It also has
a REG_CFA_DEF_CFA with (plus (sp) (const_int ...)) in lieu of a
REG_FRAME_RELATED_EXPR of the form (set (sp) (plus (...) (const_int ...))),
which like I say wouldn't always be correct. And I think that's a valid
use of REG_CFA_DEF_CFA:
/* Attached to insns that are RTX_FRAME_RELATED_P, but are too complex
for FRAME_RELATED_EXPR intuition. ... */
REG_NOTE (CFA_DEF_CFA)
(although the assignment to %r15 isn't first in the parallel, despite
the snipped part of the comment implying that it should be. I don't
understand why we'd require that though.)
FWIW this form comes from:
2009-06-04 Jakub Jelinek <jakub@redhat.com>
* config/s390/s390.c (global_not_special_regno_p): New static inline.
(save_gprs): Don't tell unwinder when a global register is saved.
(s390_emit_epilogue): Emit needed epilogue unwind info.
>> >> The simplest fix seems to be to disable this check for the exit block.
>> >> We never use its stack_adjust anyway, and dwarf2cfi already checks
>> >> (using CFA information) that the offsets in a shrink-wrapped function
>> >> are consistent.
>> >>
>> >> Tested on s390-linux-gnu and s390x-linux-gnu. OK to install?
>> >
>> > I don't like this, my strong preference is to handle REG_CFA_* notes.
>>
>> 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?
> And, var-tracking disabling doesn't really mean no debug info, just worse
> debug info. IMHO the sanity check in var-tracking is worth much more than
> var-tracking in unwind-dw2.o in the case where you wouldn't use frame
> pointer. Why doesn't dwarf2cfi ICE on it then when the CFA changes can't be
> described properly?
Because the CFA information (as provided by REG_CFA... notes) is correct.
It's just that var-tracking doesn't use those notes. FWIW, using them was
one of the options I mentioned in the original mail.
I don't see why you think relaxing this sanity check for the exit block
is so bad though. If your argument is that var-tracking must understand
every assignment to the stack pointer then I think it should bail out
whenever it sees an assignment to sp that it doesn't understand,
rather than silently ignoring it.
Thanks,
Richard