Re: var-tracking and s390's LM(G)

[Richard Sandiford <rdsandiford at googlemail 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