diff mbox

[1/2] SH epilogue unwind, dwarf2 pass changes

Message ID 4FD63454.6090000@redhat.com
State New
Headers show

Commit Message

Richard Henderson June 11, 2012, 6:09 p.m. UTC 
On 2012-06-11 01:34, Chung-Lin Tang wrote:
> Ping?
> 
> On 2012/6/1 06:24 PM, Chung-Lin Tang wrote:
>> On 12/5/23 1:46 AM, Richard Henderson wrote:
>>> On 05/18/12 03:48, Chung-Lin Tang wrote:
>>>> @@ -2401,6 +2401,7 @@ scan_trace (dw_trace_info *trace)
>>>>  	{
>>>>  	  /* Propagate across fallthru edges.  */
>>>>  	  dwarf2out_flush_queued_reg_saves ();
>>>> +	  def_cfa_1 (&this_cfa);
>>>>  	  maybe_record_trace_start (insn, NULL);
>>>>  	  break;
>>>>  	}
>>>> @@ -2455,6 +2456,18 @@ scan_trace (dw_trace_info *trace)
>>>>  		  cur_cfa = &this_cfa;
>>>>  		  continue;
>>>>  		}
>>>> +	      else
>>>> +		{
>>>> +		  /* If ELT is a annulled branch-taken instruction (i.e. executed
>>>> +		     only when branch is not taken), the args_size and CFA should
>>>> +		     not change through the jump.  */
>>>> +		  create_trace_edges (control);
>>>> +
>>>> +		  /* Update and continue with the trace.  */
>>>> +		  add_cfi_insn = insn;
>>>> +		  scan_insn_after (elt);
>>>> +		  continue;
>>>> +		}
>>>
>>> I think the def_cfa_1 is misplaced.  It should be immediately before
>>> that last continue.  That mirrors the sort of flow you get via the
>>> other paths through the loop.
>>
>> Or possibly moved before the dwarf2out_flush_queued_reg_saves () call in
>> the patch? (in the save_point_p () break case)  Note I'm only saying
>> this based on overall ordering of those two routine calls in the loop.
>>
>> Attached is a testcase (adapted from libgomp) that, with the SH epilogue
>> unwind patch applied alone, produces the ICE I'm seeing (-O2
>> -funwind-tables).
>>
>> This dwarf2 patch, with any of the three new def_cfa_1() call sites
>> seems to solve it, though you might want to comment on which call site
>> seems "correct"

I've committed the following, after a --enable-languages=c,c++ --target=sh-elf
build and check-gcc with your patch 2/2 applied.  I eyeballed the resulting
debug info and it looked correct.

Given the test case was extracted from libgomp I didn't commit that, trusting
that the real testing will continue to happen during a build.


r~
* dwarf2cfi.c (scan_trace): Handle annulled branch-taken delay slots.
diff mbox

Patch

diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c
index bf2d802f..3edb6e1 100644
--- a/gcc/dwarf2cfi.c
+++ b/gcc/dwarf2cfi.c
@@ -2429,18 +2429,20 @@  scan_trace (dw_trace_info *trace)
 
 	      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;
 		  cfi_vec save_row_reg_save;
 
+		  /* 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.  */
 		  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);
+		  save_row_reg_save
+		    = VEC_copy (dw_cfi_ref, gc, cur_row->reg_save);
 
 		  scan_insn_after (elt);
 
@@ -2453,8 +2455,20 @@  scan_trace (dw_trace_info *trace)
 		  cur_row->cfa = this_cfa;
 		  cur_row->reg_save = save_row_reg_save;
 		  cur_cfa = &this_cfa;
-		  continue;
 		}
+	      else
+		{
+		  /* If ELT is a annulled branch-taken instruction (i.e.
+		     executed only when branch is not taken), the args_size
+		     and CFA should not change through the jump.  */
+		  create_trace_edges (control);
+
+		  /* Update and continue with the trace.  */
+		  add_cfi_insn = insn;
+		  scan_insn_after (elt);
+		  def_cfa_1 (&this_cfa);
+		}
+	      continue;
 	    }
 
 	  /* The insns in the delay slot should all be considered to happen