Jakub Jelinek <ja...@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 <ja...@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