diff mbox series

combine: Correct comment about combine_validate_cost

Message ID 20250415041957.E6ADB20436@pchp3.se.axis.com
State New
Headers
Series combine: Correct comment about combine_validate_cost |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap pending Patch applied
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed

Commit Message

Hans-Peter Nilsson April 15, 2025, 4:19 a.m. UTC 
  Noticed while investigating a regression for cris-elf with
r15-9239-g4d7a634f6d4102 "combine: Allow 2->2 combinations,
but with a tweak [PR116398]" (to-be-reported).

The comment was introduced when breaking out the
combine_validate_cost function, in r0-59417-g64b8935d4809f3.

I thought about wordsmithing to keep the "polarity" of the
statement, but "are equal to or cheaper than" didn't read
well.

Ok to commit?

-- >8 --
The *code* has been the same since forever, but this
comment, at a critical path, is misleading: if the new cost
is the same (like, when doing an identity replacement), then
combine_validate_cost returns true.

	* combine.cc (try_combine): Correct comment about
	combine_validate_cost.
---
 gcc/combine.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Richard Sandiford April 15, 2025, 8:23 a.m. UTC | #1 
Hans-Peter Nilsson <hp@axis.com> writes:
> Noticed while investigating a regression for cris-elf with
> r15-9239-g4d7a634f6d4102 "combine: Allow 2->2 combinations,
> but with a tweak [PR116398]" (to-be-reported).
>
> The comment was introduced when breaking out the
> combine_validate_cost function, in r0-59417-g64b8935d4809f3.
>
> I thought about wordsmithing to keep the "polarity" of the
> statement, but "are equal to or cheaper than" didn't read
> well.
>
> Ok to commit?

OK, thanks.

Richard

> -- >8 --
> The *code* has been the same since forever, but this
> comment, at a critical path, is misleading: if the new cost
> is the same (like, when doing an identity replacement), then
> combine_validate_cost returns true.
>
> 	* combine.cc (try_combine): Correct comment about
> 	combine_validate_cost.
> ---
>  gcc/combine.cc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/combine.cc b/gcc/combine.cc
> index 5f085187cfef..c2c1d50ca49f 100644
> --- a/gcc/combine.cc
> +++ b/gcc/combine.cc
> @@ -4129,8 +4129,8 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
>  	}
>      }
>  
> -  /* Only allow this combination if insn_cost reports that the
> -     replacement instructions are cheaper than the originals.  */
> +  /* Reject this combination if insn_cost reports that the replacement
> +     instructions are more expensive than the originals.  */
>    if (!combine_validate_cost (i0, i1, i2, i3, newpat, newi2pat, other_pat))
>      {
>        undo_all ();
diff mbox series

Patch

diff --git a/gcc/combine.cc b/gcc/combine.cc
index 5f085187cfef..c2c1d50ca49f 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -4129,8 +4129,8 @@  try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
 	}
     }
 
-  /* Only allow this combination if insn_cost reports that the
-     replacement instructions are cheaper than the originals.  */
+  /* Reject this combination if insn_cost reports that the replacement
+     instructions are more expensive than the originals.  */
   if (!combine_validate_cost (i0, i1, i2, i3, newpat, newi2pat, other_pat))
     {
       undo_all ();