Re: [PATCH] Fix ARM dwarf2cfi ICE and unwind info issues (PR target/59575)
On 01/30/2014 12:38 PM, Jakub Jelinek wrote: 2014-01-30 Jakub Jelinek ja...@redhat.com PR target/59575 * config/arm/arm.c (emit_multi_reg_push): Add dwarf_regs_mask argument, don't record in REG_FRAME_RELATED_EXPR registers not set in that bitmask. (arm_expand_prologue): Adjust all callers. (arm_unwind_emit_sequence): Allow saved, but not important for unwind info, registers also at the lowest numbered registers side. Use gcc_assert instead of abort, and SET_SRC/SET_DEST macros instead of XEXP. * gcc.target/arm/pr59575.c: New test. Give the ARM guys another 24 hours to comment, but as I said in the PR I think the patch is good. r~
Re: [PATCH] Fix ARM dwarf2cfi ICE and unwind info issues (PR target/59575)
On Thu, Jan 30, 2014 at 8:38 PM, Jakub Jelinek ja...@redhat.com wrote: Hi! For -Os, apparently ARM backend sometimes decides to push (up to 8?) dummy registers to stack in addition to the registers that actually need to be saved, in order to avoid extra instruction to adjust stack pointer. These registers shouldn't be mentioned for .debug_frame though (both because it breaks assumptions of dwarf2cfi and because it doesn't make sense), they are either call clobbered which don't need saving (r0-r3) or for r4-r7 would be dummy if they aren't ever modified in the current function, again, there is no point in saving them. Even for ARM unwind info I think it doesn't make sense to mention them in the unwind info, the patch instead adds .pad #NNN directive after the .save to adjust sp correspondingly. Kyrill has kindly bootstrapped/regtested this on arm, ok for trunk? This is OK with a small comment on top of arm_unwind_emit_sequence just describing the reasoning here. I was trying to convince myself that the changes for the unwind information was safe and yes it follows similar to what you describe. regards Ramana 2014-01-30 Jakub Jelinek ja...@redhat.com PR target/59575 * config/arm/arm.c (emit_multi_reg_push): Add dwarf_regs_mask argument, don't record in REG_FRAME_RELATED_EXPR registers not set in that bitmask. (arm_expand_prologue): Adjust all callers. (arm_unwind_emit_sequence): Allow saved, but not important for unwind info, registers also at the lowest numbered registers side. Use gcc_assert instead of abort, and SET_SRC/SET_DEST macros instead of XEXP. * gcc.target/arm/pr59575.c: New test. --- gcc/config/arm/arm.c.jj 2014-01-29 10:21:11.448031616 +0100 +++ gcc/config/arm/arm.c2014-01-29 15:32:22.246381587 +0100 @@ -177,7 +177,7 @@ static rtx arm_expand_builtin (tree, rtx static tree arm_builtin_decl (unsigned, bool); static void emit_constant_insn (rtx cond, rtx pattern); static rtx emit_set_insn (rtx, rtx); -static rtx emit_multi_reg_push (unsigned long); +static rtx emit_multi_reg_push (unsigned long, unsigned long); static int arm_arg_partial_bytes (cumulative_args_t, enum machine_mode, tree, bool); static rtx arm_function_arg (cumulative_args_t, enum machine_mode, @@ -19573,28 +19573,33 @@ arm_emit_strd_push (unsigned long saved_ /* Generate and emit an insn that we will recognize as a push_multi. Unfortunately, since this insn does not reflect very well the actual semantics of the operation, we need to annotate the insn for the benefit - of DWARF2 frame unwind information. */ + of DWARF2 frame unwind information. DWARF_REGS_MASK is a subset of + MASK for registers that should be annotated for DWARF2 frame unwind + information. */ static rtx -emit_multi_reg_push (unsigned long mask) +emit_multi_reg_push (unsigned long mask, unsigned long dwarf_regs_mask) { int num_regs = 0; - int num_dwarf_regs; + int num_dwarf_regs = 0; int i, j; rtx par; rtx dwarf; int dwarf_par_index; rtx tmp, reg; + /* We don't record the PC in the dwarf frame information. */ + dwarf_regs_mask = ~(1 PC_REGNUM); + for (i = 0; i = LAST_ARM_REGNUM; i++) -if (mask (1 i)) - num_regs++; +{ + if (mask (1 i)) + num_regs++; + if (dwarf_regs_mask (1 i)) + num_dwarf_regs++; +} gcc_assert (num_regs num_regs = 16); - - /* We don't record the PC in the dwarf frame information. */ - num_dwarf_regs = num_regs; - if (mask (1 PC_REGNUM)) -num_dwarf_regs--; + gcc_assert ((dwarf_regs_mask ~mask) == 0); /* For the body of the insn we are going to generate an UNSPEC in parallel with several USEs. This allows the insn to be recognized @@ -19660,14 +19665,13 @@ emit_multi_reg_push (unsigned long mask) gen_rtvec (1, reg), UNSPEC_PUSH_MULT)); - if (i != PC_REGNUM) + if (dwarf_regs_mask (1 i)) { tmp = gen_rtx_SET (VOIDmode, gen_frame_mem (SImode, stack_pointer_rtx), reg); RTX_FRAME_RELATED_P (tmp) = 1; - XVECEXP (dwarf, 0, dwarf_par_index) = tmp; - dwarf_par_index++; + XVECEXP (dwarf, 0, dwarf_par_index++) = tmp; } break; @@ -19682,7 +19686,7 @@ emit_multi_reg_push (unsigned long mask) XVECEXP (par, 0, j) = gen_rtx_USE (VOIDmode, reg); - if (i != PC_REGNUM) + if (dwarf_regs_mask (1 i)) { tmp = gen_rtx_SET (VOIDmode, @@ -20689,7 +20693,7 @@ arm_expand_prologue (void) /* Interrupt functions must not corrupt any registers. Creating a
Re: [PATCH] Fix ARM dwarf2cfi ICE and unwind info issues (PR target/59575)
Hi Jakub, 2014-01-30 Jakub Jelinek ja...@redhat.com PR target/59575 * config/arm/arm.c (emit_multi_reg_push): Add dwarf_regs_mask argument, don't record in REG_FRAME_RELATED_EXPR registers not set in that bitmask. (arm_expand_prologue): Adjust all callers. (arm_unwind_emit_sequence): Allow saved, but not important for unwind info, registers also at the lowest numbered registers side. Use gcc_assert instead of abort, and SET_SRC/SET_DEST macros instead of XEXP. * gcc.target/arm/pr59575.c: New test. Approved - please apply. Cheers Nick
[PATCH] Fix ARM dwarf2cfi ICE and unwind info issues (PR target/59575)
Hi! For -Os, apparently ARM backend sometimes decides to push (up to 8?) dummy registers to stack in addition to the registers that actually need to be saved, in order to avoid extra instruction to adjust stack pointer. These registers shouldn't be mentioned for .debug_frame though (both because it breaks assumptions of dwarf2cfi and because it doesn't make sense), they are either call clobbered which don't need saving (r0-r3) or for r4-r7 would be dummy if they aren't ever modified in the current function, again, there is no point in saving them. Even for ARM unwind info I think it doesn't make sense to mention them in the unwind info, the patch instead adds .pad #NNN directive after the .save to adjust sp correspondingly. Kyrill has kindly bootstrapped/regtested this on arm, ok for trunk? 2014-01-30 Jakub Jelinek ja...@redhat.com PR target/59575 * config/arm/arm.c (emit_multi_reg_push): Add dwarf_regs_mask argument, don't record in REG_FRAME_RELATED_EXPR registers not set in that bitmask. (arm_expand_prologue): Adjust all callers. (arm_unwind_emit_sequence): Allow saved, but not important for unwind info, registers also at the lowest numbered registers side. Use gcc_assert instead of abort, and SET_SRC/SET_DEST macros instead of XEXP. * gcc.target/arm/pr59575.c: New test. --- gcc/config/arm/arm.c.jj 2014-01-29 10:21:11.448031616 +0100 +++ gcc/config/arm/arm.c2014-01-29 15:32:22.246381587 +0100 @@ -177,7 +177,7 @@ static rtx arm_expand_builtin (tree, rtx static tree arm_builtin_decl (unsigned, bool); static void emit_constant_insn (rtx cond, rtx pattern); static rtx emit_set_insn (rtx, rtx); -static rtx emit_multi_reg_push (unsigned long); +static rtx emit_multi_reg_push (unsigned long, unsigned long); static int arm_arg_partial_bytes (cumulative_args_t, enum machine_mode, tree, bool); static rtx arm_function_arg (cumulative_args_t, enum machine_mode, @@ -19573,28 +19573,33 @@ arm_emit_strd_push (unsigned long saved_ /* Generate and emit an insn that we will recognize as a push_multi. Unfortunately, since this insn does not reflect very well the actual semantics of the operation, we need to annotate the insn for the benefit - of DWARF2 frame unwind information. */ + of DWARF2 frame unwind information. DWARF_REGS_MASK is a subset of + MASK for registers that should be annotated for DWARF2 frame unwind + information. */ static rtx -emit_multi_reg_push (unsigned long mask) +emit_multi_reg_push (unsigned long mask, unsigned long dwarf_regs_mask) { int num_regs = 0; - int num_dwarf_regs; + int num_dwarf_regs = 0; int i, j; rtx par; rtx dwarf; int dwarf_par_index; rtx tmp, reg; + /* We don't record the PC in the dwarf frame information. */ + dwarf_regs_mask = ~(1 PC_REGNUM); + for (i = 0; i = LAST_ARM_REGNUM; i++) -if (mask (1 i)) - num_regs++; +{ + if (mask (1 i)) + num_regs++; + if (dwarf_regs_mask (1 i)) + num_dwarf_regs++; +} gcc_assert (num_regs num_regs = 16); - - /* We don't record the PC in the dwarf frame information. */ - num_dwarf_regs = num_regs; - if (mask (1 PC_REGNUM)) -num_dwarf_regs--; + gcc_assert ((dwarf_regs_mask ~mask) == 0); /* For the body of the insn we are going to generate an UNSPEC in parallel with several USEs. This allows the insn to be recognized @@ -19660,14 +19665,13 @@ emit_multi_reg_push (unsigned long mask) gen_rtvec (1, reg), UNSPEC_PUSH_MULT)); - if (i != PC_REGNUM) + if (dwarf_regs_mask (1 i)) { tmp = gen_rtx_SET (VOIDmode, gen_frame_mem (SImode, stack_pointer_rtx), reg); RTX_FRAME_RELATED_P (tmp) = 1; - XVECEXP (dwarf, 0, dwarf_par_index) = tmp; - dwarf_par_index++; + XVECEXP (dwarf, 0, dwarf_par_index++) = tmp; } break; @@ -19682,7 +19686,7 @@ emit_multi_reg_push (unsigned long mask) XVECEXP (par, 0, j) = gen_rtx_USE (VOIDmode, reg); - if (i != PC_REGNUM) + if (dwarf_regs_mask (1 i)) { tmp = gen_rtx_SET (VOIDmode, @@ -20689,7 +20693,7 @@ arm_expand_prologue (void) /* Interrupt functions must not corrupt any registers. Creating a frame pointer however, corrupts the IP register, so we must push it first. */ - emit_multi_reg_push (1 IP_REGNUM); + emit_multi_reg_push (1 IP_REGNUM, 1 IP_REGNUM); /* Do not set RTX_FRAME_RELATED_P on this insn. The dwarf stack unwinding code only wants to see one @@ -20750,7 +20754,8 @@ arm_expand_prologue (void) if (cfun-machine-uses_anonymous_args)