> From: Richard Sandiford <richard.sandif...@arm.com>
> Date: Tue, 15 Apr 2025 09:23:21 +0100

> > Ok to commit?
> 
> OK, thanks.

Thanks!

Though, I noticed another "cheaper" in the function header.
Fixing that one was a more obvious correction (thus
committed as such), as per the commit message: what the
function determines isn't whether replacement are *cheaper*,
it's whether they are *more expensive*.  Can't help but
wonder about the consequences of trusting those comments.
Since the rewording in the previous patch was approved, I
amended this change and committed the combination.

-- >8 --
Fix misleading comments.  That function only determines whether
replacements cost more; it doesn't actually *validate* costs as being
cheaper.

For example, it returns true also if it for various reasons cannot
determine the costs, or if the new cost is the same, like when doing
an identity replacement.  The code has been the same since
r0-59417-g64b8935d4809f3.

        * combine.cc: Correct comments about combine_validate_cost.
---
 gcc/combine.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/combine.cc b/gcc/combine.cc
index 5f085187cfef..e1186087dff4 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -815,7 +815,7 @@ do_SUBST_LINK (struct insn_link **into, struct insn_link 
*newval)
 #define SUBST_LINK(oldval, newval) do_SUBST_LINK (&oldval, newval)
 
 /* Subroutine of try_combine.  Determine whether the replacement patterns
-   NEWPAT, NEWI2PAT and NEWOTHERPAT are cheaper according to insn_cost
+   NEWPAT, NEWI2PAT and NEWOTHERPAT are more expensive according to insn_cost
    than the original sequence I0, I1, I2, I3 and undobuf.other_insn.  Note
    that I0, I1 and/or NEWI2PAT may be NULL_RTX.  Similarly, NEWOTHERPAT and
    undobuf.other_insn may also both be NULL_RTX.  Return false if the cost
@@ -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 ();
-- 
2.30.2

Reply via email to