Re: [PATCH 2/9] [doloop] Correct extracting loop exit condition

[Richard Sandiford]

zhr...@ispras.ru writes:
> This patch fixes the compiler segfault found while regtesting trunk with SMS 
> on
> IA64 platform.  Segfault happens on test gcc.dg/pr45259.c with -fmodulo-sched
> enabled.  The following jump instruction is given as argument for
> doloop_condition_get function:
> (jump_insn 86 85 88 7 (set (pc)
>         (reg/f:DI 403)) 339 {indirect_jump}
>      (expr_list:REG_DEAD (reg/f:DI 403)
>         (nil)))
> The patch adds checking for the form of comparison instruction before
> extracting loop exit condition.
>
> 2011-07-20  Roman Zhuykov  <zhr...@ispras.ru>
>       * loop-doloop.c (doloop_condition_get): Correctly check
>       the form of comparison instruction.
> ---
>  gcc/loop-doloop.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/gcc/loop-doloop.c b/gcc/loop-doloop.c
> index f8429c4..dfc4a16 100644
> --- a/gcc/loop-doloop.c
> +++ b/gcc/loop-doloop.c
> @@ -153,6 +153,8 @@ doloop_condition_get (rtx doloop_pat)
>        else
>          inc = PATTERN (prev_insn);
>        /* We expect the condition to be of the form (reg != 0)  */
> +      if (GET_CODE (cmp) != SET || GET_CODE (SET_SRC (cmp)) != IF_THEN_ELSE)
> +     return 0;
>        cond = XEXP (SET_SRC (cmp), 0);
>        if (GET_CODE (cond) != NE || XEXP (cond, 1) != const0_rtx)
>          return 0;

I think it'd be better to integrate:

      /* We expect the condition to be of the form (reg != 0)  */
      cond = XEXP (SET_SRC (cmp), 0);
      if (GET_CODE (cond) != NE || XEXP (cond, 1) != const0_rtx)
        return 0;

into:

  /* We expect a GE or NE comparison with 0 or 1.  */
  if ((GET_CODE (condition) != GE
       && GET_CODE (condition) != NE)
      || (XEXP (condition, 1) != const0_rtx
          && XEXP (condition, 1) != const1_rtx))
    return 0;

The next "if" already uses "GET_CODE (pattern) != PARALLEL" as a check
for the second and third cases.  E.g. something like:

  if (GET_CODE (pattern) == PARALLEL)
    {
      /* We expect a GE or NE comparison with 0 or 1.  */
      if ((GET_CODE (condition) != GE
           && GET_CODE (condition) != NE)
          || (XEXP (condition, 1) != const0_rtx
              && XEXP (condition, 1) != const1_rtx))
        return 0;
    }
  else
    {
      /* In the second and third cases, we expect the condition to
         be of the form (reg != 0)  */
      if (GET_CODE (condition) != NE || XEXP (condition, 1) != const0_rtx)
        return 0;
    }

That's pre-approved (independently of the other patches) if it works.

Richard