Patchwork PR51271. Fix handling of frame-related insn in delay slot of annulled branch

login
register
mail settings
Submitter Tom de Vries
Date Jan. 8, 2012, 12:50 p.m.
Message ID <4F09911D.3010800@mentor.com>
Download mbox | patch
Permalink /patch/134905/
State New
Headers show

Comments

Tom de Vries - Jan. 8, 2012, 12:50 p.m.
Richard,

this patch fixes PR51271. It fixes the handling in scan_trace of an insn in the
delay slot of an annulled branch that annuls the insn when the branch is not taken.

The way the insn is handled, is that:
- the insn is applied to the state
- the updated state is registered at the jump target
- the state is reset to the original state, and we continue processing at the
  fall-through path

the current related code in scan_insn is this:
...
	  if (JUMP_P (control) && INSN_ANNULLED_BRANCH_P (control))
	    {
	      /* ??? Hopefully multiple delay slots are not annulled.  */
	      gcc_assert (n == 2);
	      gcc_assert (!RTX_FRAME_RELATED_P (control));
	      gcc_assert (!find_reg_note (control, REG_ARGS_SIZE, NULL));

	      elt = XVECEXP (pat, 0, 1);

	      /* If ELT is an instruction from target of an annulled branch,
		 the effects are for the target only and so the args_size
		 and CFA along the current path shouldn't change.  */
	      if (INSN_FROM_TARGET_P (elt))
		{
		  HOST_WIDE_INT restore_args_size;

		  add_cfi_insn = NULL;
		  restore_args_size = cur_trace->end_true_args_size;
		  cur_cfa = &cur_row->cfa;

		  scan_insn_after (elt);

		  /* ??? Should we instead save the entire row state?  */
		  gcc_assert (!VEC_length (queued_reg_save, queued_reg_saves));

		  create_trace_edges (control);

		  cur_trace->end_true_args_size = restore_args_size;
		  cur_row->cfa = this_cfa;
		  cur_cfa = &this_cfa;
		  continue;
		}
	    }
...

The problem is that the state is not sufficiently restored, and this causes an
ICE in maybe_record_trace_start. For the example from PR51271, the original
state is this:
...
    .cfi_def_cfa 29, 16
    .cfi_offset 28, -8
...

and after applying the following insn:
...
(insn/s/f 143 79 162 (set (reg/f:SI 29 $sp)
        (plus:SI (reg/f:SI 29 $sp)
            (const_int 16 [0x10]))) 10 {*addsi3}
     (expr_list:REG_CFA_DEF_CFA (reg/f:SI 29 $sp)
        (expr_list:REG_CFA_RESTORE (reg:DI 28 $28)
            (nil))))
...

the state is updated to this and applied to the target:
...
    .cfi_def_cfa 29, 0
...

but after restoring the state it is not the original one:
...
    .cfi_def_cfa 29, 16
...

the patch additionally saves and restores cur_row->reg_save, which makes sure
that the state is restored to the original state, and prevents the
maybe_record_trace_start ICE.

build & reg-tested on mips64el-linux-gnu.

OK for trunk?

Thanks,
- Tom


2012-01-08  Tom de Vries  <tom@codesourcery.com>

	* dwarf2cfi.c (scan_trace): Save and restore cur_row->reg_save when
	handling annulled branch.
Richard Henderson - Jan. 10, 2012, 2:25 a.m.
On 01/08/2012 11:50 PM, Tom de Vries wrote:
> 	* dwarf2cfi.c (scan_trace): Save and restore cur_row->reg_save when
> 	handling annulled branch.

Ok.


r~

Patch

Index: gcc/dwarf2cfi.c
===================================================================
--- gcc/dwarf2cfi.c (revision 182903)
+++ gcc/dwarf2cfi.c (working copy)
@@ -2439,10 +2439,12 @@  scan_trace (dw_trace_info *trace)
 	      if (INSN_FROM_TARGET_P (elt))
 		{
 		  HOST_WIDE_INT restore_args_size;
+		  cfi_vec save_row_reg_save;
 
 		  add_cfi_insn = NULL;
 		  restore_args_size = cur_trace->end_true_args_size;
 		  cur_cfa = &cur_row->cfa;
+		  save_row_reg_save = VEC_copy (dw_cfi_ref, gc, cur_row->reg_save);
 
 		  scan_insn_after (elt);
 
@@ -2453,6 +2455,7 @@  scan_trace (dw_trace_info *trace)
 
 		  cur_trace->end_true_args_size = restore_args_size;
 		  cur_row->cfa = this_cfa;
+		  cur_row->reg_save = save_row_reg_save;
 		  cur_cfa = &this_cfa;
 		  continue;
 		}