PATCH for [ARM] subsequent use of plus and minus operators could be improved

[Zack Weinberg]

Gábor Lóki <alga@rgai.hu> writes:

> Richard Earnshaw wrote:
>  > First of all, please read the documents on contributing to GCC and pay
>  > particular attention to the coding standards sections. Patches can only
>  > be accepted if they conform to these standards.
>
> Ok. I am done with the code forming.

This is looking good.  A couple of style nitpicks:

> +       /* Try to transform 
> +            (set (regX) (plus\minus (regY) (constA)))
> +            (set (regX) (plus\minus (regX) (constB)))
> +          to
> +            (set (regX) (plus\minus (regY) (constA +\- constB))) */

You should use forward slashes (/) here, not backslashes (\).  

> +       if (next 
> +           && GET_CODE (pat) == SET 
> +           && GET_CODE (next) == INSN 
> +           && GET_CODE (PATTERN (next)) == SET)
> +         {
> +           rtx insn_dest = SET_DEST (pat);
> +           rtx insn_src = SET_SRC (pat);
> +           rtx next_dest = SET_DEST (PATTERN (next));
> +           rtx next_src = SET_SRC (PATTERN (next));

You should probably use single_set here.

> +               /* Combine the two constant into a new one.  */
> +               rtx new_value = GEN_INT (INTVAL (XEXP (insn_src, 1)) + 
> +                                        sign * INTVAL (XEXP (next_src, 1)));

English grammar: "Combine the two constants into a new one."  Or just
delete this comment; it's not really necessary.


> +               if (new_value == const0_rtx)
> +               /* Create (set (regX) (regY)) if the new constant is zero.  */
> +                 success = validate_change (insn, 

This comment should be indented to line up with the word "success",
and word-wrapped to fit in 80 columns:

                  if (new_value == const0_rtx)
                    /* Create (set (regX) (regY)) if the new 
                       constant is zero.  */
                    success = validate_change (insn, 


> +               /* Override the constA with the new constant.  */

English grammar: "Override constA with the new constant."  Better
still, "Otherwise, replace constA with the new constant."

> I examined the reload_cse_move2add candidate again, but as I saw it
> first, this function tries to transform move instruction into
> add. This isn't what I'd like to do, and no other reload_cse passes do
> the same thing. These were the reasons why I created a new function.
> You are right. There is no reason for another scan on the RTL. So I
> merged my code with reload_cse_move2add. You can see in the attached
> patch.

Perhaps it is appropriate to rename reload_cse_move2add - call it
"postreload_combine" or "postreload_simplify_rtx" or something like
that.  Other people can make better suggestions than me.

zw