[PATCH] -ftree-loop-linear fixes (PR tree-optimization/46970)

[Sebastian Pop]

On Fri, Dec 17, 2010 at 17:11, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> gcc_loop_to_lambda_loop doesn't reject both when loop exit condition's
> SSA_NAME defined in the loop has defining statement a phi, or
> some statement where a single ssa use of it has defining statement a phi,
> but actually seems to expect only the latter case, because for the new iv
> it uses in the exit condition the incremented iv instead of the phi result.
>
> Additionally, it doesn't check whether the statement really in the middle
> is an increment by step, nor whether step isn't e.g. 0x100000001ULL when
> it stores into a host int LL_STEP field.
>
> This patch tries to fix that.  The upper bound adjustment still looks unsafe
> to me in case it overflows, but that has been a problem before the patch and
> is not solved by the patch and would probably need much bigger changes in
> lambda-code.c.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux.
>
> 2010-12-17  Jakub Jelinek  <jakub@redhat.com>
>
>        PR tree-optimization/46970
>        * lambda-code.c (gcc_loop_to_lambda_loop): Give up if
>        step doesn't fit into host int or if def stmt of inductionvar
>        is neither PHI nor increment by step.  If exit condition
>        compares induction variable before increment, adjust ubound
>        differently.
>
>        * gcc.dg/pr46970-1.c: New test.
>        * gcc.dg/pr46970-2.c: New test.
>

The patch looks good to me, but I cannot approve it.

>   /* We might have some leftover.  */
> -  if (gimple_cond_code (exit_cond) == LT_EXPR)
> -    extra = -1 * stepint;
> -  else if (gimple_cond_code (exit_cond) == NE_EXPR)
> -    extra = -1 * stepint;
> -  else if (gimple_cond_code (exit_cond) == GT_EXPR)
> -    extra = -1 * stepint;
> -  else if (gimple_cond_code (exit_cond) == EQ_EXPR)
> -    extra = 1 * stepint;
> +  if (SSA_NAME_DEF_STMT (inductionvar) != phi)
> +    {
> +      if (gimple_cond_code (exit_cond) == LT_EXPR)
> +       extra = -1 * stepint;
> +      else if (gimple_cond_code (exit_cond) == NE_EXPR)
> +       extra = -1 * stepint;
> +      else if (gimple_cond_code (exit_cond) == GT_EXPR)
> +       extra = -1 * stepint;
> +      else if (gimple_cond_code (exit_cond) == EQ_EXPR)
> +       extra = 1 * stepint;
> +    }
> +  else
> +    {
> +      if (gimple_cond_code (exit_cond) == LE_EXPR)
> +       extra = 1 * stepint;
> +      else if (gimple_cond_code (exit_cond) == GE_EXPR)
> +       extra = 1 * stepint;
> +    }

It would be easier to read the following equivalent code:

if (SSA_NAME_DEF_STMT (inductionvar) != phi)
  {
    enum tree_code code = gimple_cond_code (exit_cond);

    if (code == LT_EXPR
        || code == NE_EXPR
	|| code == GT_EXPR)
     extra = -1 * stepint;
    else if (code == EQ_EXPR)
     extra = 1 * stepint;
  }
else
  {
    enum tree_code code = gimple_cond_code (exit_cond);

    if (code == LE_EXPR
        || code == GE_EXPR)
     extra = 1 * stepint;
  }