Fix infinite recursion in simplify_binary_operation

[Richard Guenther]

On Mon, Sep 1, 2008 at 8:57 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> A MIPS patch I'm working on caused us to try to optimise the rather
> odd combination:
>
>    (plus (plus (high ...) [== X]
>                (const_int 4) [== Y])
>          ...)
>
> and this led to infinite recursion between simplify_binary_operation
> and simplify_plus_minus.  The latter function has some special code
> to avoid this:
>
>                /* Reject "simplifications" that just wrap the two
>                   arguments in a CONST.  Failure to do so can result
>                   in infinite recursion with simplify_binary_operation
>                   when it calls us to simplify CONST operations.  */
>                if (tem
>                    && ! (GET_CODE (tem) == CONST
>                          && GET_CODE (XEXP (tem, 0)) == ncode
>                          && XEXP (XEXP (tem, 0), 0) == lhs
>                          && XEXP (XEXP (tem, 0), 1) == rhs))
>                  {
>                    lneg &= rneg;
>                    if (GET_CODE (tem) == NEG)
>                      tem = XEXP (tem, 0), lneg = !lneg;
>                    if (GET_CODE (tem) == CONST_INT && lneg)
>                      tem = neg_const_int (mode, tem), lneg = 0;
>
>                    ops[i].op = tem;
>                    ops[i].neg = lneg;
>                    ops[j].op = NULL_RTX;
>                    changed = 1;
>                  }
>
> Unfortunately, in this case, simplify_binary_operation (PLUS, Pmode, X, Y)
> is returning (plus:P X Y) rather than (const:P (plus:P X Y)), which isn't
> a simplification at all.  (const:P (plus:P X Y)) isn't valid rtl, so the
> return value ought to be NULL instead.
>
> The problem is with the following code:
>
>      if (CONSTANT_P (op0) && GET_MODE (op0) != VOIDmode
>          && GET_CODE (op1) == CONST_INT)
>        return plus_constant (op0, INTVAL (op1));
>      else if (CONSTANT_P (op1) && GET_MODE (op1) != VOIDmode
>               && GET_CODE (op0) == CONST_INT)
>        return plus_constant (op1, INTVAL (op0));
>
> plus_constant only returns a CONST for three codes: CONST, SYMBOL_REF
> and LABEL_REF, so those are the only types of rtx we should simplify.
>
> I wondered about adding a "symbolic_constant_p" predicate, but not all
> target-dependent CONSTs are strictly symbolic.  The condition here is
> more tied to plus_constant than to an abstract concept.
>
> Tested on x86_64-linux-gnu and mips64-linux-gnu.  OK to install?

Ok.

Thanks,
Richard.

> Richard
>
>
> gcc/
>        * simplify-rtx.c (simplify_binary_operation_1): Check for CONST,
>        SYMBOL_REF and LABEL_REF when applying plus_constant, instead of
>        checking for a non-VOID CONSTANT_P.
>
> Index: gcc/simplify-rtx.c
> ===================================================================
> --- gcc/simplify-rtx.c  2008-08-30 14:10:15.000000000 +0100
> +++ gcc/simplify-rtx.c  2008-08-30 16:08:40.000000000 +0100
> @@ -1594,10 +1594,14 @@ simplify_binary_operation_1 (enum rtx_co
>         to CONST_INT since overflow won't be computed properly if wider
>         than HOST_BITS_PER_WIDE_INT.  */
>
> -      if (CONSTANT_P (op0) && GET_MODE (op0) != VOIDmode
> +      if ((GET_CODE (op0) == CONST
> +          || GET_CODE (op0) == SYMBOL_REF
> +          || GET_CODE (op0) == LABEL_REF)
>          && GET_CODE (op1) == CONST_INT)
>        return plus_constant (op0, INTVAL (op1));
> -      else if (CONSTANT_P (op1) && GET_MODE (op1) != VOIDmode
> +      else if ((GET_CODE (op1) == CONST
> +               || GET_CODE (op1) == SYMBOL_REF
> +               || GET_CODE (op1) == LABEL_REF)
>               && GET_CODE (op0) == CONST_INT)
>        return plus_constant (op1, INTVAL (op0));
>
>