Most comments, including the second sentence in the head comment
of combine_validate_cost, the main decision-maker of the combine
pass, refer to the function as returning true if the new
insns(s) *cheaper* than the old insns, when in fact the function
returned true also if the cost was the same.  Returning true for
cheaper also seems more sane than as-cheap-as considering the
need to avoid oscillation between same-cost combinations.  Also,
it makes the job of later passes harder, having combine make
more complex combinations of the same cost.

Right, you can affect this with your target TARGET_RTX_COSTS and
TARGET_INSN_COST hooks, but only for trivial cases, and you have
increasingly more complex combinations (many-to-many
combinations) where you have to twist simple logic to appease
combine (stop it from combining) or give up.

Main-interest ports are unsurprisingly pretty tied to this
effect.  I'd love to install the following patch, adjusting the
function and the two opposing comments.  But...it causes
hundreds of regressions for each of x86_64-linux and
aarch64-linux (tens for ppc64le-linux), so I'm just not up to
the task, at least not without previous buy-in from reviewers.

It would need those targets to have their TARGET_INSN_COST
and/or TARGET_RTX_COSTS functions adjusted.

Alternatives from the top of my head, one of:

- With buy-in from global reviewers, installing this patch on a
development branch and let all target maintainers adjust their
target test-cases and cost-functions there, for merge when
first-class targets are done.  (I'm a dreamer.)

- A target combine hook for the decision (passing for inspection
tuples of from-insns and to-insns and costs) and just falling
back to the current addition of rtx costs.

- A simpler target combine decision hook that says which one of
"cheaper" or "as-cheap-as".

- Adjusting documentation and comments that are currently
untruthful about the cost decision to instead say (to the effect
of) "as cheap as" instead of "cheaper".

So, WDYT?

(Tested as above, causing massive pattern-match regressions.)

gcc:
        * combine.c (combine_validate_cost): Reject unless the new total
        cost is cheaper than the original.  Adjust the minority of
        comments that don't say "cheaper":

---
 gcc/combine.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gcc/combine.c b/gcc/combine.c
index f69413a..7da144e 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -846,8 +846,8 @@ do_SUBST_LINK (struct insn_link **into, struct insn_link 
*newval)
    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
-   of all the instructions can be estimated and the replacements are more
-   expensive than the original sequence.  */
+   of all the instructions can be estimated and the replacements are not
+   cheaper than the original sequence.  */
 
 static bool
 combine_validate_cost (rtx_insn *i0, rtx_insn *i1, rtx_insn *i2, rtx_insn *i3,
@@ -938,8 +938,8 @@ combine_validate_cost (rtx_insn *i0, rtx_insn *i1, rtx_insn 
*i2, rtx_insn *i3,
     }
 
   /* Disallow this combination if both new_cost and old_cost are greater than
-     zero, and new_cost is greater than old cost.  */
-  int reject = old_cost > 0 && new_cost > old_cost;
+     zero, and new_cost is greater than or equal to the old cost.  */
+  int reject = old_cost > 0 && new_cost >= old_cost;
 
   if (dump_file)
     {
-- 
2.11.0

Reply via email to