Re: RFA: Fix loop-iv-related regression in pr27285.c

[Richard Guenther <richard dot guenther at gmail dot com>]

On Thu, Feb 19, 2009 at 9:21 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Several places in loop-iv.c want to truncate an iv from extend_mode to a
> smaller mode.  The problem is that they unconditionally do this using
> simplify_gen_subreg, which is incorrect for !TRULY_NOOP_TRUNCATION
> conversions.  This causes gcc.c-torture/execute/pr27285.c to fail on
> mipsisa64-elf with the options "-O3 -funroll-loops".
>
> The simple fix is to use simplify_gen_unary (TRUNCATE, ...) instead.
> simplify_gen_unary knows when it can use subregs, so the change doesn't
> pessmise sane targets.  (To be sure, I verified that the patch did
> not change the assembly output for the C and C++ testsuites on either
> x86_64-linux-gnu or i686-pc-linux-gnu, using the options -O3
> -funroll-all-loops.)  I think the change is conceptually cleaner too.
>
> While loop-iv.c no longer needs the lowpart_subreg function itself,
> it exported this function to various other rtl passes and targets.
> I therefore moved it into emit-rtl.c instead of deleting it.
>
> Tested on mipsisa64-elf.  Also tested with a C-only bootstrap on
> x86_64-linux-gnu; a full test is now in progress.  OK to install
> if the bootstrap succeeds?  (Sorry for asking before waiting,
> but I realise we're close to branching.)

Ok.

Thanks,
Richard.

> Richard
>
>
> gcc/
>        * loop-iv.c (truncate_value): New function.
>        (iv_subreg, get_iv_value, iv_number_of_iterations): Use it instead
>        of lowpart_subreg.
>        (lowpart_subreg): Move to...
>        * emit-rtl.c: ...here.
>
> Index: gcc/loop-iv.c
> ===================================================================
> --- gcc/loop-iv.c       2009-02-17 20:22:34.000000000 +0000
> +++ gcc/loop-iv.c       2009-02-19 19:09:41.000000000 +0000
> @@ -156,15 +156,12 @@ dump_iv_info (FILE *file, struct rtx_iv
>     fprintf (file, " (first special)");
>  }
>
> -/* Generates a subreg to get the least significant part of EXPR (in mode
> -   INNER_MODE) to OUTER_MODE.  */
> +/* Truncate EXPR (which has mode EXPR_MODE) to MODE.  */
>
> -rtx
> -lowpart_subreg (enum machine_mode outer_mode, rtx expr,
> -               enum machine_mode inner_mode)
> +static rtx
> +truncate_value (enum machine_mode mode, rtx expr, enum machine_mode expr_mode)
>  {
> -  return simplify_gen_subreg (outer_mode, expr, inner_mode,
> -                             subreg_lowpart_offset (outer_mode, inner_mode));
> +  return simplify_gen_unary (TRUNCATE, mode, expr, expr_mode);
>  }
>
>  static void
> @@ -407,7 +404,7 @@ iv_subreg (struct rtx_iv *iv, enum machi
>       && !iv->first_special)
>     {
>       rtx val = get_iv_value (iv, const0_rtx);
> -      val = lowpart_subreg (mode, val, iv->extend_mode);
> +      val = truncate_value (mode, val, iv->extend_mode);
>
>       iv->base = val;
>       iv->extend = UNKNOWN;
> @@ -1246,7 +1243,7 @@ get_iv_value (struct rtx_iv *iv, rtx ite
>   if (iv->extend_mode == iv->mode)
>     return val;
>
> -  val = lowpart_subreg (iv->mode, val, iv->extend_mode);
> +  val = truncate_value (iv->mode, val, iv->extend_mode);
>
>   if (iv->extend == UNKNOWN)
>     return val;
> @@ -2199,8 +2196,8 @@ iv_number_of_iterations (struct loop *lo
>   mode = iv0.mode;
>   size = GET_MODE_BITSIZE (mode);
>   get_mode_bounds (mode, (cond == LE || cond == LT), comp_mode, &mmin, &mmax);
> -  mode_mmin = lowpart_subreg (mode, mmin, comp_mode);
> -  mode_mmax = lowpart_subreg (mode, mmax, comp_mode);
> +  mode_mmin = truncate_value (mode, mmin, comp_mode);
> +  mode_mmax = truncate_value (mode, mmax, comp_mode);
>
>   if (GET_CODE (iv0.step) != CONST_INT || GET_CODE (iv1.step) != CONST_INT)
>     goto fail;
> @@ -2257,7 +2254,7 @@ iv_number_of_iterations (struct loop *lo
>           difficult due to possibly infinite loops.  */
>        if (iv0.step == const0_rtx)
>          {
> -           tmp = lowpart_subreg (mode, iv0.base, comp_mode);
> +           tmp = truncate_value (mode, iv0.base, comp_mode);
>            assumption = simplify_gen_relational (EQ, SImode, mode, tmp,
>                                                  mode_mmax);
>            if (assumption == const_true_rtx)
> @@ -2267,7 +2264,7 @@ iv_number_of_iterations (struct loop *lo
>          }
>        else
>          {
> -           tmp = lowpart_subreg (mode, iv1.base, comp_mode);
> +           tmp = truncate_value (mode, iv1.base, comp_mode);
>            assumption = simplify_gen_relational (EQ, SImode, mode, tmp,
>                                                  mode_mmin);
>            if (assumption == const_true_rtx)
> @@ -2293,7 +2290,7 @@ iv_number_of_iterations (struct loop *lo
>     {
>       if (iv0.step == const0_rtx)
>        {
> -         tmp = lowpart_subreg (mode, iv0.base, comp_mode);
> +         tmp = truncate_value (mode, iv0.base, comp_mode);
>          if (rtx_equal_p (tmp, mode_mmin))
>            {
>              desc->infinite =
> @@ -2304,7 +2301,7 @@ iv_number_of_iterations (struct loop *lo
>        }
>       else
>        {
> -         tmp = lowpart_subreg (mode, iv1.base, comp_mode);
> +         tmp = truncate_value (mode, iv1.base, comp_mode);
>          if (rtx_equal_p (tmp, mode_mmax))
>            {
>              desc->infinite =
> @@ -2328,7 +2325,7 @@ iv_number_of_iterations (struct loop *lo
>       else
>        step = iv0.step;
>       delta = simplify_gen_binary (MINUS, comp_mode, iv1.base, iv0.base);
> -      delta = lowpart_subreg (mode, delta, comp_mode);
> +      delta = truncate_value (mode, delta, comp_mode);
>       delta = simplify_gen_binary (UMOD, mode, delta, step);
>       may_xform = const0_rtx;
>       may_not_xform = const_true_rtx;
> @@ -2352,8 +2349,8 @@ iv_number_of_iterations (struct loop *lo
>            {
>              bound = simplify_gen_binary (PLUS, comp_mode, mmin, step);
>              bound = simplify_gen_binary (MINUS, comp_mode, bound, delta);
> -             bound = lowpart_subreg (mode, bound, comp_mode);
> -             tmp = lowpart_subreg (mode, iv0.base, comp_mode);
> +             bound = truncate_value (mode, bound, comp_mode);
> +             tmp = truncate_value (mode, iv0.base, comp_mode);
>              may_xform = simplify_gen_relational (cond, SImode, mode,
>                                                   bound, tmp);
>              may_not_xform = simplify_gen_relational (reverse_condition (cond),
> @@ -2364,8 +2361,8 @@ iv_number_of_iterations (struct loop *lo
>            {
>              bound = simplify_gen_binary (MINUS, comp_mode, mmax, step);
>              bound = simplify_gen_binary (PLUS, comp_mode, bound, delta);
> -             bound = lowpart_subreg (mode, bound, comp_mode);
> -             tmp = lowpart_subreg (mode, iv1.base, comp_mode);
> +             bound = truncate_value (mode, bound, comp_mode);
> +             tmp = truncate_value (mode, iv1.base, comp_mode);
>              may_xform = simplify_gen_relational (cond, SImode, mode,
>                                                   tmp, bound);
>              may_not_xform = simplify_gen_relational (reverse_condition (cond),
> @@ -2416,8 +2413,8 @@ iv_number_of_iterations (struct loop *lo
>              iv1.base = simplify_gen_binary (PLUS, comp_mode, iv1.base, step);
>            }
>
> -         tmp0 = lowpart_subreg (mode, iv0.base, comp_mode);
> -         tmp1 = lowpart_subreg (mode, iv1.base, comp_mode);
> +         tmp0 = truncate_value (mode, iv0.base, comp_mode);
> +         tmp1 = truncate_value (mode, iv1.base, comp_mode);
>          assumption = simplify_gen_relational (reverse_condition (cond),
>                                                SImode, mode, tmp0, tmp1);
>          if (assumption == const_true_rtx)
> @@ -2445,7 +2442,7 @@ iv_number_of_iterations (struct loop *lo
>          iv0.step = simplify_gen_unary (NEG, comp_mode, iv0.step, mode);
>          iv1.base = simplify_gen_unary (NEG, comp_mode, iv1.base, mode);
>        }
> -      iv0.step = lowpart_subreg (mode, iv0.step, comp_mode);
> +      iv0.step = truncate_value (mode, iv0.step, comp_mode);
>
>       /* Let nsd (s, size of mode) = d.  If d does not divide c, the loop
>         is infinite.  Otherwise, the number of iterations is
> @@ -2459,7 +2456,7 @@ iv_number_of_iterations (struct loop *lo
>        }
>       bound = GEN_INT (((unsigned HOST_WIDEST_INT) 1 << (size - 1 ) << 1) - 1);
>
> -      tmp1 = lowpart_subreg (mode, iv1.base, comp_mode);
> +      tmp1 = truncate_value (mode, iv1.base, comp_mode);
>       tmp = simplify_gen_binary (UMOD, mode, tmp1, GEN_INT (d));
>       assumption = simplify_gen_relational (NE, SImode, mode, tmp, const0_rtx);
>       desc->infinite = alloc_EXPR_LIST (0, assumption, desc->infinite);
> @@ -2480,12 +2477,11 @@ iv_number_of_iterations (struct loop *lo
>           but it was already taken into account during LE -> NE transform).  */
>        {
>          step = iv0.step;
> -         tmp0 = lowpart_subreg (mode, iv0.base, comp_mode);
> -         tmp1 = lowpart_subreg (mode, iv1.base, comp_mode);
> +         tmp0 = truncate_value (mode, iv0.base, comp_mode);
> +         tmp1 = truncate_value (mode, iv1.base, comp_mode);
>
>          bound = simplify_gen_binary (MINUS, mode, mode_mmax,
> -                                      lowpart_subreg (mode, step,
> -                                                      comp_mode));
> +                                      truncate_value (mode, step, comp_mode));
>          if (step_is_pow2)
>            {
>              rtx t0, t1;
> @@ -2512,7 +2508,7 @@ iv_number_of_iterations (struct loop *lo
>            }
>
>          tmp = simplify_gen_binary (PLUS, comp_mode, iv1.base, iv0.step);
> -         tmp = lowpart_subreg (mode, tmp, comp_mode);
> +         tmp = truncate_value (mode, tmp, comp_mode);
>          assumption = simplify_gen_relational (reverse_condition (cond),
>                                                SImode, mode, tmp0, tmp);
>
> @@ -2525,11 +2521,11 @@ iv_number_of_iterations (struct loop *lo
>             We must know that a - s does not overflow and a - s <= b and then
>             we can again compute number of iterations as (b - (a - s)) / s.  */
>          step = simplify_gen_unary (NEG, mode, iv1.step, mode);
> -         tmp0 = lowpart_subreg (mode, iv0.base, comp_mode);
> -         tmp1 = lowpart_subreg (mode, iv1.base, comp_mode);
> +         tmp0 = truncate_value (mode, iv0.base, comp_mode);
> +         tmp1 = truncate_value (mode, iv1.base, comp_mode);
>
>          bound = simplify_gen_binary (PLUS, mode, mode_mmin,
> -                                      lowpart_subreg (mode, step, comp_mode));
> +                                      truncate_value (mode, step, comp_mode));
>          if (step_is_pow2)
>            {
>              rtx t0, t1;
> @@ -2556,7 +2552,7 @@ iv_number_of_iterations (struct loop *lo
>            }
>
>          tmp = simplify_gen_binary (PLUS, comp_mode, iv0.base, iv1.step);
> -         tmp = lowpart_subreg (mode, tmp, comp_mode);
> +         tmp = truncate_value (mode, tmp, comp_mode);
>          assumption = simplify_gen_relational (reverse_condition (cond),
>                                                SImode, mode,
>                                                tmp, tmp1);
> Index: gcc/emit-rtl.c
> ===================================================================
> --- gcc/emit-rtl.c      2009-02-17 20:22:34.000000000 +0000
> +++ gcc/emit-rtl.c      2009-02-19 19:09:41.000000000 +0000
> @@ -1232,6 +1232,17 @@ gen_lowpart_common (enum machine_mode mo
>   /* Otherwise, we can't do this.  */
>   return 0;
>  }
> +
> +/* Generates a subreg to get the least significant part of EXPR (in mode
> +   INNER_MODE) to OUTER_MODE.  */
> +
> +rtx
> +lowpart_subreg (enum machine_mode outer_mode, rtx expr,
> +               enum machine_mode inner_mode)
> +{
> +  return simplify_gen_subreg (outer_mode, expr, inner_mode,
> +                             subreg_lowpart_offset (outer_mode, inner_mode));
> +}
>
>  rtx
>  gen_highpart (enum machine_mode mode, rtx x)
>