[Bug debug/103619] armeb ICE since r12-5833

["cvs-commit at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103619

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:e75a0a03588977c8c758091f9b50d456a5f67227

commit r12-5994-ge75a0a03588977c8c758091f9b50d456a5f67227
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Dec 15 10:41:02 2021 +0100

    dwarf2cfi: Improve cfa_reg comparisons [PR103619]

    On Tue, Dec 14, 2021 at 10:32:21AM -0700, Jeff Law wrote:
    > I think the attached testcase should trigger on c6x with -mbig-endian -O2
-g

    Thanks.  Finally I see what's going on.  c6x doesn't really need the CFA
    with span > 1 (and I bet neither does armbe), the only reason why
    dwf_cfa_reg is called is that the code in 13 cases just tries to compare
    the CFA against dwf_cfa_reg (some_reg).  And that dwf_cfa_reg on some reg
    that usually isn't a CFA reg results in targetm.dwarf_register_span hook
    call, which on targets like c6x or armeb and others for some registers
    creates a PARALLEL with various REGs in it, then the loop with the
assertion
    and finally operator== which just notes that the reg is different and
fails.

    This seems compile time memory and time inefficient.

    The following so far untested patch instead adds an extra operator== and !=
    for comparison of cfa_reg with rtx, which has the most common case where it
    is a different register number done early without actually invoking
    dwf_cfa_reg.  This means the assertion in dwf_cfa_reg can stay as is (at
    least until some big endian target needs to have hard frame pointer or
stack
    pointer with span > 1 as well).
    I've removed a different assertion there because it is redundant -
dwf_regno
    already has exactly that assertion in it too.

    And I've included those 2 tweaks to avoid creating a REG in GC memory when
    we can use {stack,hard_frame}_pointer_rtx which is already initialized to
    the same REG we need by init_emit_regs.

    On Tue, Dec 14, 2021 at 03:05:37PM -0700, Jeff Law wrote:
    > 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)

    2021-12-15  Jakub Jelinek  <jakub@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).