Re: [PATCH] rtl: add predicates for addition, subtraction & multiplication

[Xi Ruoyao <xry111@xry111.site>]

On Sat, 2022-11-26 at 21:16 -0500, Charlie Sale via Gcc-patches wrote:
> This is my first contribution to GCC :) one of the beginner projects
> suggested on the website was to add and use RTL type predicates. I
> added predicates for addition, subtraction and multiplication. I also
> went through and used them in the code.
> 
> I did not add tests because I'm not addding/modifying any behavior.
> All existings tests did pass.
> 
> Like I said, this is my first patch. Please let me know if I did
> anything wrong or if there's anything I can improve for next time.
> 
> Signed-off-by: Charlie Sale <softwaresale01@gmail.com>
> ---
>  gcc/ChangeLog            |  43 +++++++
>  gcc/alias.cc             |  30 +++--
>  gcc/auto-inc-dec.cc      |  11 +-
>  gcc/calls.cc             |   8 +-
>  gcc/cfgexpand.cc         |  16 +--
>  gcc/combine-stack-adj.cc |  39 +++----
>  gcc/combine.cc           | 241 +++++++++++++++++----------------------
>  gcc/compare-elim.cc      |   3 +-
>  gcc/cse.cc               |  66 +++++------
>  gcc/cselib.cc            |  37 +++---
>  gcc/dce.cc               |   4 +-
>  gcc/dwarf2cfi.cc         |   2 +-
>  gcc/dwarf2out.cc         |  11 +-
>  gcc/emit-rtl.cc          |   6 +-
>  gcc/explow.cc            |  31 ++---
>  gcc/expr.cc              |  23 ++--
>  gcc/final.cc             |  20 ++--
>  gcc/function.cc          |   7 +-
>  gcc/fwprop.cc            |   2 +-
>  gcc/haifa-sched.cc       |  10 +-
>  gcc/ifcvt.cc             |  11 +-
>  gcc/ira.cc               |   6 +-
>  gcc/loop-doloop.cc       |  70 ++++++------
>  gcc/loop-iv.cc           |  21 +---
>  gcc/lra-constraints.cc   |  34 +++---
>  gcc/lra-eliminations.cc  |  25 ++--
>  gcc/lra.cc               |   6 +-
>  gcc/modulo-sched.cc      |   2 +-
>  gcc/postreload.cc        |  25 ++--
>  gcc/reginfo.cc           |  12 +-
>  gcc/reload.cc            | 180 +++++++++++++----------------
>  gcc/reload1.cc           |  85 ++++++--------
>  gcc/reorg.cc             |  12 +-
>  gcc/rtl.cc               |   3 +-
>  gcc/rtl.h                |  11 ++
>  gcc/rtlanal.cc           |  25 ++--
>  gcc/sched-deps.cc        |   8 +-
>  gcc/simplify-rtx.cc      | 143 +++++++++--------------
>  gcc/var-tracking.cc      |  37 +++---
>  39 files changed, 595 insertions(+), 731 deletions(-)
> 
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog

Do not edit this file.  It's automatically generated from Git commit
log.  Write the ChangeLog entries in the commit message instead.
 
/* snip */

> -             return simplify_gen_binary (GET_CODE (x), GET_MODE (x),
> -                                         op0, XEXP (x, 1));
> +             return simplify_gen_binary (GET_CODE (x), GET_MODE (x), op0,
> +                                         XEXP (x, 1));

Don't update things irrelevant to your topic stealthy.

/* snip */

> -                              && (t = get_reg_known_value (REGNO (XEXP (src, 0))))
> +                              && (t = get_reg_known_value (
> +                                    REGNO (XEXP (src, 0))))

Likewise.

/* snip */

> -             op0 = expand_debug_expr (TREE_OPERAND (TREE_OPERAND (exp, 0),
> -                                                    0));
> +             op0 = expand_debug_expr (TREE_OPERAND (TREE_OPERAND (exp, 0), 0));

Likewise.

/* snip */

> -         else if (dest == stack_pointer_rtx
> -                  && REG_P (src)
> +         else if (dest == stack_pointer_rtx && REG_P (src)

Likewise, and GCC has its own coding style which is not "what looks
better in your opinion".

I'll stop reading the patch here because it's too long and there is
already too much stealth changes.

Try to break the patch into a series of multiple small patches because
it's difficult to review a very large diff.

Do not randomly modify coding style.  Even if you have a good reason to
make style changes, you should submit them as a separate patch (or
separate patch series) because mixing style changes and real code
changes makes it difficult to review the code.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University