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

[Chung-Lin Tang <cltang at codesourcery dot com>]

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"
> 
> Thanks,
> Chung-Lin