On Tue, Dec 14, 2021 at 03:05:37PM -0700, Jeff Law wrote:
> > 2021-12-14 Jakub Jelinek <ja...@redhat.com>
> >
> > PR debug/103619
> > * dwarf2cfi.c (dwf_cfa_reg): Remove gcc_assert.
> > (operator==, operator!=): New overloaded operators.
> > (dwarf2out_frame_debug_adjust_cfa, dwarf2out_frame_debug_cfa_offset,
> > dwarf2out_frame_debug_expr): Compare vars with cfa_reg type directly
> > with REG rtxes rather than with dwf_cfa_reg results on those REGs.
> > (create_cie_data): Use stack_pointer_rtx instead of
> > gen_rtx_REG (Pmode, STACK_POINTER_REGNUM).
> > (execute_dwarf2_frame): Use hard_frame_pointer_rtx instead of
> > gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM).
> So if someone is unfamiliar with the underlying issues here and needs to
> twiddle dwarf2cfi, how are they supposed to know if they should compare
> directly or use dwf_cfa_reg?
Comparison without dwf_cfa_reg should be used whenever possible, because
for registers which are never CFA related that won't call
targetm.dwarf_register_span uselessly.
The only comparisons with dwf_cfa_reg I've kept are the:
regno = dwf_cfa_reg (XEXP (XEXP (dest, 0), 0));
if (cur_cfa->reg == regno)
offset -= cur_cfa->offset;
else if (cur_trace->cfa_store.reg == regno)
offset -= cur_trace->cfa_store.offset;
else
{
gcc_assert (cur_trace->cfa_temp.reg == regno);
offset -= cur_trace->cfa_temp.offset;
}
and
struct cfa_reg regno = dwf_cfa_reg (XEXP (dest, 0));
if (cur_cfa->reg == regno)
offset = -cur_cfa->offset;
else if (cur_trace->cfa_store.reg == regno)
offset = -cur_trace->cfa_store.offset;
else
{
gcc_assert (cur_trace->cfa_temp.reg == regno);
offset = -cur_trace->cfa_temp.offset;
}
and there are 2 reasons for it:
1) there is an assertion, which guarantees it must compare equal to one of
those 3 cfa related struct cfa_reg structs, so it must be some CFA related
register (so, right now, targetm.dwarf_register_span shouldn't return
non-NULL in those on anything but gcn)
2) it is compared 3 times in a row, so for the GCN case doing
if (cur_cfa->reg == XEXP (XEXP (dest, 0), 0))
offset -= cur_cfa->offset;
else if (cur_trace->cfa_store.reg == XEXP (XEXP (dest, 0), 0))
offset -= cur_trace->cfa_store.offset;
else
{
gcc_assert (cur_trace->cfa_temp.reg == XEXP (XEXP (dest, 0),
0));
offset -= cur_trace->cfa_temp.offset;
}
could actually create more GC allocated garbage than the way it is written
now. But doing it that way would work fine.
I think for most of the comparisons even comparing with dwf_cfa_reg would
work but be less compile time/memory efficient (e.g. those assertions that
it is equal to some CFA related cfa_reg or in any spots where only the CFA
related regs may appear in the frame related patterns).
I'm aware just of a single spot where comparison with dwf_cfa_reg doesn't
work (when the assert is in dwf_cfa_reg), that is the spot that was ICEing
on your testcase, where we save arbitrary call saved register:
if (REG_P (src)
&& REGNO (src) != STACK_POINTER_REGNUM
&& REGNO (src) != HARD_FRAME_POINTER_REGNUM
&& cur_cfa->reg == src)
Jakub
