https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126328

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #6)
> (In reply to Segher Boessenkool from comment #5)
> > (In reply to Richard Biener from comment #3)
> > > so we are back to a cost of 4, because combine_validate_cost doesn't
> > > re-recog before costing :/
> > 
> > What does this mean?  The result of recog () should depend only on the RTL 
> > of
> > the insn, always.  So why would you ever want to run recog () more often 
> > than
> > required?
> 
> Because you want to write the insn matching code once.  Like you could
> add a 'cost' insn attribute to patterns in the .md file and in the
> insn_cost hook simply return that.  But - combine calls insn_cost when
> the insn isn't recognized.  Now, I suppose the hook isn't supposed to modify
> the insn (even though it doesnt' get a const_rtx_insn *), or do you say
> the insn_cost hook should simply call recog () to match up a pattern with
> costs?
> 
> See https://gcc.gnu.org/pipermail/gcc-patches/2026-July/724831.html
> how I propose to "fix" the x86 insn_cost hook to recognize the hadd
> patterns.  Relying on recog () would be way easier and more maintainable?

I'll note that it seems try_combine already recognized i3, the insn code
is in insn_code_number!  Just combine_validate_cost doesn't use it?  So
why not

diff --git a/gcc/combine.cc b/gcc/combine.cc
index 53ea168a44b..23744e37b62 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -825,7 +825,8 @@ do_SUBST_LINK (struct insn_link **into, struct insn_link
*ne
wval)

 static bool
 combine_validate_cost (rtx_insn *i0, rtx_insn *i1, rtx_insn *i2, rtx_insn *i3,
-                      rtx newpat, rtx newi2pat, rtx newotherpat)
+                      int insn_code_number, rtx newpat, rtx newi2pat,
+                      rtx newotherpat)
 {
   int i0_cost, i1_cost, i2_cost, i3_cost;
   int new_i2_cost, new_i3_cost;
@@ -867,7 +868,7 @@ combine_validate_cost (rtx_insn *i0, rtx_insn *i1, rtx_insn
*i2, rtx_insn *i3,
   rtx tmp = PATTERN (i3);
   PATTERN (i3) = newpat;
   int tmpi = INSN_CODE (i3);
-  INSN_CODE (i3) = -1;
+  INSN_CODE (i3) = insn_code_number;
   new_i3_cost = insn_cost (i3, optimize_this_for_speed_p);
   PATTERN (i3) = tmp;
   INSN_CODE (i3) = tmpi;
@@ -4134,7 +4135,8 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1,
rtx_insn *i0,

   /* 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))
+  if (!combine_validate_cost (i0, i1, i2, i3, insn_code_number,
+                             newpat, newi2pat, other_pat))
     {
       undo_all ();
       return 0;

Reply via email to