[PATCH] Fix bootstrap failure because of -Wreturn-local-addr

[PATCH] Fix bootstrap failure because of -Wreturn-local-addr

[Jakub Jelinek]

Hi!

I've tried to bootstrap with r213515 because later revisions broke because
of the hash_map and Ada changes, but unfortunately even that revision
failus to bootstrap, the new -Wreturn-local-addr warning rightfully
warns about get_ivts_expr possibly returning address of a local variable.

It seems that n_loc is always 1 and loc[0] is always 1, already since
4.0 when these loop-unroll.c changes have been introduced, so I think the
best fix is just to remove those two fields.

Bootstrapped/regtested (with r213515, rtl checking) on x86_64-linux and
i686-linux, ok for trunk?

2014-08-04  Jakub Jelinek  <jakub@redhat.com>

	* loop-unroll.c (struct iv_to_split): Remove n_loc and loc fields.
	(analyze_iv_to_split_insn): Don't initialize them.
	(get_ivts_expr): Removed.
	(allocate_basic_variable, insert_base_initialization): Use
	SET_SRC instead of *get_ivts_expr.
	(split_iv): Use &SET_SRC instead of get_ivts_expr.

--- gcc/loop-unroll.c.jj	2014-06-24 16:41:55.000000000 +0200
+++ gcc/loop-unroll.c	2014-08-04 14:13:35.750917507 +0200
@@ -79,11 +79,6 @@ struct iv_to_split
 			   iterations are based.  */
   rtx step;		/* Step of the induction variable.  */
   struct iv_to_split *next; /* Next entry in walking order.  */
-  unsigned n_loc;
-  unsigned loc[3];	/* Location where the definition of the induction
-			   variable occurs in the insn.  For example if
-			   N_LOC is 2, the expression is located at
-			   XEXP (XEXP (single_set, loc[0]), loc[1]).  */
 };
 
 /* Information about accumulators to expand.  */
@@ -1942,8 +1937,6 @@ analyze_iv_to_split_insn (rtx insn)
   ivts->base_var = NULL_RTX;
   ivts->step = iv.step;
   ivts->next = NULL;
-  ivts->n_loc = 1;
-  ivts->loc[0] = 1;
 
   return ivts;
 }
@@ -2080,27 +2073,12 @@ determine_split_iv_delta (unsigned n_cop
     }
 }
 
-/* Locate in EXPR the expression corresponding to the location recorded
-   in IVTS, and return a pointer to the RTX for this location.  */
-
-static rtx *
-get_ivts_expr (rtx expr, struct iv_to_split *ivts)
-{
-  unsigned i;
-  rtx *ret = &expr;
-
-  for (i = 0; i < ivts->n_loc; i++)
-    ret = &XEXP (*ret, ivts->loc[i]);
-
-  return ret;
-}
-
 /* Allocate basic variable for the induction variable chain.  */
 
 static void
 allocate_basic_variable (struct iv_to_split *ivts)
 {
-  rtx expr = *get_ivts_expr (single_set (ivts->insn), ivts);
+  rtx expr = SET_SRC (single_set (ivts->insn));
 
   ivts->base_var = gen_reg_rtx (GET_MODE (expr));
 }
@@ -2111,7 +2089,7 @@ allocate_basic_variable (struct iv_to_sp
 static void
 insert_base_initialization (struct iv_to_split *ivts, rtx insn)
 {
-  rtx expr = copy_rtx (*get_ivts_expr (single_set (insn), ivts));
+  rtx expr = copy_rtx (SET_SRC (single_set (insn)));
   rtx seq;
 
   start_sequence ();
@@ -2146,7 +2124,7 @@ split_iv (struct iv_to_split *ivts, rtx
     }
 
   /* Figure out where to do the replacement.  */
-  loc = get_ivts_expr (single_set (insn), ivts);
+  loc = &SET_SRC (single_set (insn));
 
   /* If we can make the replacement right away, we're done.  */
   if (validate_change (insn, loc, expr, 0))

	Jakub

Re: [PATCH] Fix bootstrap failure because of -Wreturn-local-addr

[Marc Glisse]

On Mon, 4 Aug 2014, Jakub Jelinek wrote:

> I've tried to bootstrap with r213515 because later revisions broke because
> of the hash_map and Ada changes, but unfortunately even that revision
> failus to bootstrap, the new -Wreturn-local-addr warning rightfully
> warns about get_ivts_expr possibly returning address of a local variable.
>
> It seems that n_loc is always 1 and loc[0] is always 1, already since
> 4.0 when these loop-unroll.c changes have been introduced, so I think the
> best fix is just to remove those two fields.

Thanks, that will fix PR 62005.
I'll try to remember to test with RTL checking for the next warning patch.

-- 
Marc Glisse

Re: [PATCH] Fix bootstrap failure because of -Wreturn-local-addr

[Richard Biener]

On Mon, 4 Aug 2014, Jakub Jelinek wrote:

> Hi!
> 
> I've tried to bootstrap with r213515 because later revisions broke because
> of the hash_map and Ada changes, but unfortunately even that revision
> failus to bootstrap, the new -Wreturn-local-addr warning rightfully
> warns about get_ivts_expr possibly returning address of a local variable.
> 
> It seems that n_loc is always 1 and loc[0] is always 1, already since
> 4.0 when these loop-unroll.c changes have been introduced, so I think the
> best fix is just to remove those two fields.
> 
> Bootstrapped/regtested (with r213515, rtl checking) on x86_64-linux and
> i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2014-08-04  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* loop-unroll.c (struct iv_to_split): Remove n_loc and loc fields.
> 	(analyze_iv_to_split_insn): Don't initialize them.
> 	(get_ivts_expr): Removed.
> 	(allocate_basic_variable, insert_base_initialization): Use
> 	SET_SRC instead of *get_ivts_expr.
> 	(split_iv): Use &SET_SRC instead of get_ivts_expr.
> 
> --- gcc/loop-unroll.c.jj	2014-06-24 16:41:55.000000000 +0200
> +++ gcc/loop-unroll.c	2014-08-04 14:13:35.750917507 +0200
> @@ -79,11 +79,6 @@ struct iv_to_split
>  			   iterations are based.  */
>    rtx step;		/* Step of the induction variable.  */
>    struct iv_to_split *next; /* Next entry in walking order.  */
> -  unsigned n_loc;
> -  unsigned loc[3];	/* Location where the definition of the induction
> -			   variable occurs in the insn.  For example if
> -			   N_LOC is 2, the expression is located at
> -			   XEXP (XEXP (single_set, loc[0]), loc[1]).  */
>  };
>  
>  /* Information about accumulators to expand.  */
> @@ -1942,8 +1937,6 @@ analyze_iv_to_split_insn (rtx insn)
>    ivts->base_var = NULL_RTX;
>    ivts->step = iv.step;
>    ivts->next = NULL;
> -  ivts->n_loc = 1;
> -  ivts->loc[0] = 1;
>  
>    return ivts;
>  }
> @@ -2080,27 +2073,12 @@ determine_split_iv_delta (unsigned n_cop
>      }
>  }
>  
> -/* Locate in EXPR the expression corresponding to the location recorded
> -   in IVTS, and return a pointer to the RTX for this location.  */
> -
> -static rtx *
> -get_ivts_expr (rtx expr, struct iv_to_split *ivts)
> -{
> -  unsigned i;
> -  rtx *ret = &expr;
> -
> -  for (i = 0; i < ivts->n_loc; i++)
> -    ret = &XEXP (*ret, ivts->loc[i]);
> -
> -  return ret;
> -}
> -
>  /* Allocate basic variable for the induction variable chain.  */
>  
>  static void
>  allocate_basic_variable (struct iv_to_split *ivts)
>  {
> -  rtx expr = *get_ivts_expr (single_set (ivts->insn), ivts);
> +  rtx expr = SET_SRC (single_set (ivts->insn));
>  
>    ivts->base_var = gen_reg_rtx (GET_MODE (expr));
>  }
> @@ -2111,7 +2089,7 @@ allocate_basic_variable (struct iv_to_sp
>  static void
>  insert_base_initialization (struct iv_to_split *ivts, rtx insn)
>  {
> -  rtx expr = copy_rtx (*get_ivts_expr (single_set (insn), ivts));
> +  rtx expr = copy_rtx (SET_SRC (single_set (insn)));
>    rtx seq;
>  
>    start_sequence ();
> @@ -2146,7 +2124,7 @@ split_iv (struct iv_to_split *ivts, rtx
>      }
>  
>    /* Figure out where to do the replacement.  */
> -  loc = get_ivts_expr (single_set (insn), ivts);
> +  loc = &SET_SRC (single_set (insn));
>  
>    /* If we can make the replacement right away, we're done.  */
>    if (validate_change (insn, loc, expr, 0))
> 
> 	Jakub