Fix PR debug/59418

Hi,

this is a latent bug exposed on the mainline for the ARM by:
  http://gcc.gnu.org/ml/gcc-patches/2013-11/msg01755.html

The problem is that the CFI expressions for:

(insn/f:TI 102 11 103 (parallel [
            (set (mem/c:BLK (pre_modify:SI (reg/f:SI 13 sp)
                        (plus:SI (reg/f:SI 13 sp)
                            (const_int -8 [0xfffffffffffffff8]))) [2  A8])
                (unspec:BLK [
                        (reg:DF 32 s16)
                    ] UNSPEC_PUSH_MULT))
        ]) pr59418.c:5 728 {*push_multi_vfp}
     (expr_list:REG_DEAD (reg:DF 32 s16)
        (expr_list:REG_FRAME_RELATED_EXPR (sequence [
                    (set/f (reg/f:SI 13 sp)
                        (plus:SI (reg/f:SI 13 sp)
                            (const_int -8 [0xfffffffffffffff8])))
                    (set/f (mem/c:DF (reg/f:SI 13 sp) [2  S8 A64])
                        (reg:DF 32 s16))
                ])
            (nil))))

and:

(insn/f:TI 113 128 20 (parallel [
            (set/f (reg/f:SI 13 sp)
                (plus:SI (reg/f:SI 13 sp)
                    (const_int 8 [0x8])))
            (set/f (reg:DF 32 s16)
                (mem/c:DF (reg/f:SI 13 sp) [2  S8 A64]))
        ]) pr59418.c:28 347 {*vfp_pop_multiple_with_writeback}
     (expr_list:REG_CFA_ADJUST_CFA (set (reg/f:SI 13 sp)
            (plus:SI (reg/f:SI 13 sp)
                (const_int 8 [0x8])))
        (expr_list:REG_CFA_RESTORE (reg:DF 32 s16)
            (nil))))

are treated differently: for the former, the source register (reg:DF 32 s16)
goes through TARGET_DWARF_REGISTER_SPAN but, for the latter, the same register
being restored does not, which results in CFI mismatch caught by the checking.

Fixed by handling TARGET_DWARF_REGISTER_SPAN for REG_CFA_RESTORE, tested on 
arm-eabi, OK for the mainline?


2013-12-15  Eric Botcazou  <ebotcazou@adacore.com>

	PR debug/59418
	* dwarf2cfi.c (dwarf2out_frame_debug_cfa_offset): Fix comment and clean	
	up implementation.
	(dwarf2out_frame_debug_cfa_restore): Handle TARGET_DWARF_REGISTER_SPAN.
	(dwarf2out_frame_debug_expr): Clean up implementation.


2013-12-15  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc.dg/pr59418.c: New test.

On Sun, Dec 15, 2013 at 06:53:07PM +0100, Eric Botcazou wrote:
> 2013-12-15  Eric Botcazou  <ebotcazou@adacore.com>
> 
> 	PR debug/59418
> 	* dwarf2cfi.c (dwarf2out_frame_debug_cfa_offset): Fix comment and clean	
> 	up implementation.
> 	(dwarf2out_frame_debug_cfa_restore): Handle TARGET_DWARF_REGISTER_SPAN.
> 	(dwarf2out_frame_debug_expr): Clean up implementation.
> 
> 
> 2013-12-15  Eric Botcazou  <ebotcazou@adacore.com>
> 
> 	* gcc.dg/pr59418.c: New test.

> @@ -1149,18 +1149,14 @@ dwarf2out_frame_debug_cfa_offset (rtx se
>    else
>      {
>        /* We have a PARALLEL describing where the contents of SRC live.
> -   	 Queue register saves for each piece of the PARALLEL.  */
> -      int par_index;
> -      int limit;
> +   	 Adjust the offset for each piece of the PARALLEL.  */
>        HOST_WIDE_INT span_offset = offset;
>  
>        gcc_assert (GET_CODE (span) == PARALLEL);
>  
> -      limit = XVECLEN (span, 0);
> -      for (par_index = 0; par_index < limit; par_index++)
> +      for (int par_index = 0; par_index < XVECLEN (span, 0); par_index++)

Is it really a good idea to put the XVECLEN into the loop condition?
I mean, the functions that are called in the loop are unlikely pure
and thus the length will need to be uselessly reread for each iteration.

My preference would be to keep the limit hoisted manually before the loop.

>  	{
>  	  /* We have a PARALLEL describing where the contents of SRC live.
>  	     Queue register saves for each piece of the PARALLEL.  */
> -	  int par_index;
> -	  int limit;
>  	  HOST_WIDE_INT span_offset = offset;
>  
>  	  gcc_assert (GET_CODE (span) == PARALLEL);
>  
> -	  limit = XVECLEN (span, 0);
> -	  for (par_index = 0; par_index < limit; par_index++)
> +	  for (int par_index = 0; par_index < XVECLEN (span, 0); par_index++)

And here too.

Otherwise looks good to me.

	Jakub

> Is it really a good idea to put the XVECLEN into the loop condition?
> I mean, the functions that are called in the loop are unlikely pure
> and thus the length will need to be uselessly reread for each iteration.

I'm not sure we are supposed to care about this kind of micro-optimization in 
the compiler but...

> My preference would be to keep the limit hoisted manually before the loop.

...OK, I'll leave it alone.

> Otherwise looks good to me.

Thanks.