[Committed] Fix 54524: Wrong code with some 64bit addition with registers as 32bits

Hi,
  For some reason cse produced:
(insn 27 43 28 2 (set (reg:SI 231 [+4 ])
        (plus:SI (reg:SI 229 [+4 ])
            (const_int 0 [0]))) t.c:24 10 {*addsi3}
     (nil))

(insn 28 27 29 2 (set (reg:SI 212)
        (ltu:SI (reg:SI 231 [+4 ])
            (reg:SI 229 [+4 ]))) t.c:24 521 {*sltu_sisi}
     (nil))

And then forwprop goes and does the following:
In insn 28, replacing
 (ltu:SI (reg:SI 231 [+4 ])
        (reg:SI 229 [+4 ]))
 with (const_int 1 [0x1])

This is due to simplify-rtx.c (simplify_relational_operation_1) Having
the following optimization:
  /* (LTU/GEU (PLUS a C) C), where C is constant, can be simplified to
     (GEU/LTU a -C).  Likewise for (LTU/GEU (PLUS a C) a).  */

But this is wrong when C is 0.  Anyways I committed this patch as
obvious to fix the issue in simplify-rtx.c but I have not looked into
why CSE does simplify the plus.

Thanks,
Andrew Pinski

ChangeLog:

        PR rtl-opt/54524
        * simplify-rtx.c (simplify_relational_operation_1): Don't simplify
        (LTU/GEU (PLUS a 0) 0) into (GEU/LTU a 0) since they are not equivalent.

Andrew Pinski <andrew.pinski@caviumnetworks.com> writes:
> Hi,
>   For some reason cse produced:
> (insn 27 43 28 2 (set (reg:SI 231 [+4 ])
>         (plus:SI (reg:SI 229 [+4 ])
>             (const_int 0 [0]))) t.c:24 10 {*addsi3}
>      (nil))
>
> (insn 28 27 29 2 (set (reg:SI 212)
>         (ltu:SI (reg:SI 231 [+4 ])
>             (reg:SI 229 [+4 ]))) t.c:24 521 {*sltu_sisi}
>      (nil))
>
> And then forwprop goes and does the following:
> In insn 28, replacing
>  (ltu:SI (reg:SI 231 [+4 ])
>         (reg:SI 229 [+4 ]))
>  with (const_int 1 [0x1])
>
> This is due to simplify-rtx.c (simplify_relational_operation_1) Having
> the following optimization:
>   /* (LTU/GEU (PLUS a C) C), where C is constant, can be simplified to
>      (GEU/LTU a -C).  Likewise for (LTU/GEU (PLUS a C) a).  */
>
> But this is wrong when C is 0.  Anyways I committed this patch as
> obvious to fix the issue in simplify-rtx.c but I have not looked into
> why CSE does simplify the plus.

Hmm, like you said originally in the PR, I think generating the invalid
PLUS is the bug here.  I agree (as Richard B said) that an invalid PLUS
shouldn't cause us to generate wrong code, but an assert seems better
than a check.  Eric, what do you think?

Richard

>         PR rtl-opt/54524
>         * simplify-rtx.c (simplify_relational_operation_1): Don't simplify
>         (LTU/GEU (PLUS a 0) 0) into (GEU/LTU a 0) since they are not equivalent.
>
> Index: simplify-rtx.c
> ===================================================================
> --- simplify-rtx.c	(revision 193110)
> +++ simplify-rtx.c	(working copy)
> @@ -4546,7 +4546,9 @@ simplify_relational_operation_1 (enum rt
>        && GET_CODE (op0) == PLUS
>        && CONST_INT_P (XEXP (op0, 1))
>        && (rtx_equal_p (op1, XEXP (op0, 0))
> -	  || rtx_equal_p (op1, XEXP (op0, 1))))
> +	  || rtx_equal_p (op1, XEXP (op0, 1)))
> +      /* (LTU/GEU (PLUS a 0) 0) is not the same as (GEU/LTU a 0). */
> +      && XEXP (op0, 1) != const0_rtx)
>      {
>        rtx new_cmp
>  	= simplify_gen_unary (NEG, cmp_mode, XEXP (op0, 1), cmp_mode);

> Hmm, like you said originally in the PR, I think generating the invalid
> PLUS is the bug here.  I agree (as Richard B said) that an invalid PLUS
> shouldn't cause us to generate wrong code, but an assert seems better
> than a check.  Eric, what do you think?

I think that this boils down to deciding whether (PLUS X 0) is invalid or 
merely non-canonical.  If the former, then I agree that an assertion is 
appropriate; if the latter, then I don't think that we should stop the 
compiler because of it.

No strong opinion, although I'd lean towards the latter.  In any case, the 
piece of code which generates this PLUS needs also to be fixed (at least on 
the mainline).