diff options
author | Jakub Jelinek <jakub@redhat.com> | 2021-12-15 10:41:02 +0100 |
---|---|---|
committer | Jakub Jelinek <jakub@redhat.com> | 2021-12-15 10:43:44 +0100 |
commit | e75a0a03588977c8c758091f9b50d456a5f67227 (patch) | |
tree | c36ce7bd86fc83fab68875edc72484ebcf40dfd6 /gcc/dwarf2cfi.c | |
parent | 127c7178d5ec502d95862fd823537cbca1a0cb99 (diff) | |
download | gcc-e75a0a03588977c8c758091f9b50d456a5f67227.zip gcc-e75a0a03588977c8c758091f9b50d456a5f67227.tar.gz gcc-e75a0a03588977c8c758091f9b50d456a5f67227.tar.bz2 |
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).
Diffstat (limited to 'gcc/dwarf2cfi.c')
-rw-r--r-- | gcc/dwarf2cfi.c | 58 |
1 files changed, 37 insertions, 21 deletions
diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c index 9dd1dfe..b8aa805 100644 --- a/gcc/dwarf2cfi.c +++ b/gcc/dwarf2cfi.c @@ -1113,8 +1113,6 @@ dwf_cfa_reg (rtx reg) { struct cfa_reg result; - gcc_assert (REGNO (reg) < FIRST_PSEUDO_REGISTER); - result.reg = dwf_regno (reg); result.span = 1; result.span_width = 0; @@ -1144,6 +1142,28 @@ dwf_cfa_reg (rtx reg) return result; } +/* More efficient comparisons that don't call targetm.dwarf_register_span + unnecessarily. These cfa_reg vs. rtx comparisons should be done at + least for call-saved REGs that might not be CFA related (like stack + pointer, hard frame pointer or DRAP registers are), in other cases it is + just a compile time and memory optimization. */ + +static bool +operator== (cfa_reg &cfa, rtx reg) +{ + unsigned int regno = dwf_regno (reg); + if (cfa.reg != regno) + return false; + struct cfa_reg other = dwf_cfa_reg (reg); + return cfa == other; +} + +static inline bool +operator!= (cfa_reg &cfa, rtx reg) +{ + return !(cfa == reg); +} + /* Compare X and Y for equivalence. The inputs may be REGs or PC_RTX. */ static bool @@ -1313,7 +1333,7 @@ dwarf2out_frame_debug_adjust_cfa (rtx pat) switch (GET_CODE (src)) { case PLUS: - gcc_assert (dwf_cfa_reg (XEXP (src, 0)) == cur_cfa->reg); + gcc_assert (cur_cfa->reg == XEXP (src, 0)); cur_cfa->offset -= rtx_to_poly_int64 (XEXP (src, 1)); break; @@ -1346,11 +1366,11 @@ dwarf2out_frame_debug_cfa_offset (rtx set) switch (GET_CODE (addr)) { case REG: - gcc_assert (dwf_cfa_reg (addr) == cur_cfa->reg); + gcc_assert (cur_cfa->reg == addr); offset = -cur_cfa->offset; break; case PLUS: - gcc_assert (dwf_cfa_reg (XEXP (addr, 0)) == cur_cfa->reg); + gcc_assert (cur_cfa->reg == XEXP (addr, 0)); offset = rtx_to_poly_int64 (XEXP (addr, 1)) - cur_cfa->offset; break; default: @@ -1797,7 +1817,7 @@ dwarf2out_frame_debug_expr (rtx expr) { /* Setting FP from SP. */ case REG: - if (cur_cfa->reg == dwf_cfa_reg (src)) + if (cur_cfa->reg == src) { /* Rule 1 */ /* Update the CFA rule wrt SP or FP. Make sure src is @@ -1828,7 +1848,7 @@ dwarf2out_frame_debug_expr (rtx expr) { gcc_assert (REGNO (dest) == HARD_FRAME_POINTER_REGNUM && fde->drap_reg != INVALID_REGNUM - && cur_cfa->reg != dwf_cfa_reg (src) + && cur_cfa->reg != src && fde->rule18); fde->rule18 = 0; /* The save of hard frame pointer has been deferred @@ -1852,8 +1872,7 @@ dwarf2out_frame_debug_expr (rtx expr) /* Adjusting SP. */ if (REG_P (XEXP (src, 1))) { - gcc_assert (dwf_cfa_reg (XEXP (src, 1)) - == cur_trace->cfa_temp.reg); + gcc_assert (cur_trace->cfa_temp.reg == XEXP (src, 1)); offset = cur_trace->cfa_temp.offset; } else if (!poly_int_rtx_p (XEXP (src, 1), &offset)) @@ -1886,7 +1905,7 @@ dwarf2out_frame_debug_expr (rtx expr) gcc_assert (frame_pointer_needed); gcc_assert (REG_P (XEXP (src, 0)) - && dwf_cfa_reg (XEXP (src, 0)) == cur_cfa->reg); + && cur_cfa->reg == XEXP (src, 0)); offset = rtx_to_poly_int64 (XEXP (src, 1)); if (GET_CODE (src) != MINUS) offset = -offset; @@ -1899,7 +1918,7 @@ dwarf2out_frame_debug_expr (rtx expr) /* Rule 4 */ if (REG_P (XEXP (src, 0)) - && dwf_cfa_reg (XEXP (src, 0)) == cur_cfa->reg + && cur_cfa->reg == XEXP (src, 0) && poly_int_rtx_p (XEXP (src, 1), &offset)) { /* Setting a temporary CFA register that will be copied @@ -1914,7 +1933,7 @@ dwarf2out_frame_debug_expr (rtx expr) /* Rule 5 */ else if (REG_P (XEXP (src, 0)) - && dwf_cfa_reg (XEXP (src, 0)) == cur_trace->cfa_temp.reg + && cur_trace->cfa_temp.reg == XEXP (src, 0) && XEXP (src, 1) == stack_pointer_rtx) { /* Setting a scratch register that we will use instead @@ -1945,7 +1964,7 @@ dwarf2out_frame_debug_expr (rtx expr) /* Rule 7 */ case IOR: gcc_assert (REG_P (XEXP (src, 0)) - && dwf_cfa_reg (XEXP (src, 0)) == cur_trace->cfa_temp.reg + && cur_trace->cfa_temp.reg == XEXP (src, 0) && CONST_INT_P (XEXP (src, 1))); cur_trace->cfa_temp.reg = dwf_cfa_reg (dest); @@ -1981,7 +2000,7 @@ dwarf2out_frame_debug_expr (rtx expr) dwarf2out_flush_queued_reg_saves (); gcc_assert (cur_trace->cfa_store.reg - == dwf_cfa_reg (XEXP (src, 0))); + == XEXP (src, 0)); fde->stack_realign = 1; fde->stack_realignment = INTVAL (XEXP (src, 1)); cur_trace->cfa_store.offset = 0; @@ -2109,8 +2128,7 @@ dwarf2out_frame_debug_expr (rtx expr) /* Rule 14 */ case POST_INC: - gcc_assert (cur_trace->cfa_temp.reg - == dwf_cfa_reg (XEXP (XEXP (dest, 0), 0))); + gcc_assert (cur_trace->cfa_temp.reg == XEXP (XEXP (dest, 0), 0)); offset = -cur_trace->cfa_temp.offset; cur_trace->cfa_temp.offset -= GET_MODE_SIZE (GET_MODE (dest)); break; @@ -2128,7 +2146,7 @@ dwarf2out_frame_debug_expr (rtx expr) if (REG_P (src) && REGNO (src) != STACK_POINTER_REGNUM && REGNO (src) != HARD_FRAME_POINTER_REGNUM - && dwf_cfa_reg (src) == cur_cfa->reg) + && cur_cfa->reg == src) { /* We're storing the current CFA reg into the stack. */ @@ -3210,8 +3228,7 @@ create_cie_data (void) dw_cfa_location loc; dw_trace_info cie_trace; - dw_stack_pointer_regnum = dwf_cfa_reg (gen_rtx_REG (Pmode, - STACK_POINTER_REGNUM)); + dw_stack_pointer_regnum = dwf_cfa_reg (stack_pointer_rtx); memset (&cie_trace, 0, sizeof (cie_trace)); cur_trace = &cie_trace; @@ -3270,8 +3287,7 @@ static unsigned int execute_dwarf2_frame (void) { /* Different HARD_FRAME_POINTER_REGNUM might coexist in the same file. */ - dw_frame_pointer_regnum - = dwf_cfa_reg (gen_rtx_REG (Pmode, HARD_FRAME_POINTER_REGNUM)); + dw_frame_pointer_regnum = dwf_cfa_reg (hard_frame_pointer_rtx); /* The first time we're called, compute the incoming frame state. */ if (cie_cfi_vec == NULL) |