Re: [PATCH] rtl cse: Fix PR94740, ICE on testsuite/gcc.dg/sso/t5.c with -mcpu=future -mpcrel -O1

[Richard Sandiford]

Peter Bergner <berg...@linux.ibm.com> writes:
> On 4/28/20 11:57 AM, Richard Sandiford wrote:
>> I was just quoting code from simplify_replace_fn_rtx as an example of
>> something that handles a similar situation.  The recursive calls would
>> be different for cse_process_notes_1.
>
> Ok, how about the following which adds the (const:P ...) as well, which
> fixes the ICE?  The FOR loop which now follows this switch statement is
> what used to modify the PLUS's operands without adding the (const:P ).
>
> This is still in the middle of bootstrap and regtesting.
>
> Peter
>
>
> gcc/
>       PR rtl-optimization/94740
>       * cse.c (cse_process_notes_1):
>
> gcc/testsuite/
>       PR rtl-optimization/94740
>       * gcc.target/powerpc/pr94740.c: New test.
>
> diff --git a/gcc/cse.c b/gcc/cse.c
> index 5aaba8d80e0..4592820c04c 100644
> --- a/gcc/cse.c
> +++ b/gcc/cse.c
> @@ -6314,6 +6314,21 @@ cse_process_notes_1 (rtx x, rtx object, bool *changed)
>        break;
>      }
>  
> +  switch (GET_RTX_CLASS (code))
> +    {
> +    case RTX_BIN_ARITH:
> +    case RTX_COMM_ARITH:
> +      /* Call simplify_gen_binary with our updated operands in case they
> +      are now constant and need to be wrapped with a (const:P ...).  */
> +      validate_change (object, &XEXP (x, 0),
> +                    cse_process_notes (XEXP (x, 0), object, changed), false);
> +      validate_change (object, &XEXP (x, 1),
> +                    cse_process_notes (XEXP (x, 1), object, changed), false);
> +      return simplify_gen_binary (code, GET_MODE (x), XEXP (x, 0), XEXP (x, 
> 1));

If we use simplify_gen_binary then I don't think we need to validate
each change individually.  It should be enough to do something like:

      x0 = cse_process_notes (XEXP (x, 0), object, changed);
      x1 = cse_process_notes (XEXP (x, 1), object, changed);
      if (x0 == XEXP (x, 0) && x1 == XEXP (x, 1))
        return x;
      return simplify_gen_binary (code, GET_MODE (x), x0, x1);

Alternatively, in the hope of reducing redundant rtl, I guess we could
add the switch statement after the format walk and do:

  switch (GET_RTX_CLASS (code))
    {
    case RTX_BIN_ARITH:
    case RTX_COMM_ARITH:
      if (rtx new_rtx = simplify_binary (code, GET_MODE (x),
                                         XEXP (x, 0), XEXP (x, 1)))
        return new_rtx;
      break;
    default:
      break;
   }

Thanks,
Richard