[Bug rtl-optimization/106594] [13 Regression] sign-extensions no longer merged into addressing mode

["rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #18 from Richard Biener <rguenth at gcc dot gnu.org> ---
OK, sorry for chiming in here ;)  I see that expand_compound_operation handles
sign_extends that can be expressed as zero_extends as the cheaper variant of

 a) the zero_extend
 b) the original sign_extend
 c) what expand_compound_operation recursively makes out of the zero_extend

while zero_extend seems to be always expanded to shifts (if possible)
as canonicalization(?) without considering costs.

So either that canonicalization isn't a canonicalization and thus at the
end of expand_compound_operation we should cost the shift sequence against
x and return x if that was cheaper _or_ the sign_extend case should not
leak non-canonical zero_extend and thus should not consider the bare
zero_extend but only the recursively treated one (supposed the sign_extend
is fine to be kept and not canonicalized to zero_extend).

Thus the problem in combines expand_compound_operation is confusion about
canonicalization vs. transform to a cheaper variant?  The comment of
the function suggests it should do canonicalization and instead
make_compound_operation would be the more appropriate place to do
costing?  (but I see none in the latter ...)

That said, the regression is caused by alternate input to the combine pass,
not by combine itself and from the looks at the IL snippets in comment#2
it might be just nonzero-bits knowledge that differs?

So for expand_compound_operation either

diff --git a/gcc/combine.cc b/gcc/combine.cc
index 053879500b7..bb151446ef3 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -7335,6 +7335,10 @@ expand_compound_operation (rtx x)
   if (GET_CODE (tem) == CLOBBER)
     return x;

+  if (set_src_cost (tem, mode, optimize_this_for_speed_p)
+      > set_src_cost (x, mode, optimize_this_for_speed_p))
+    return x;
+
   return tem;
 }

(which should enable simplification of the sign_extend checking)

or (more radical than suggested above) pruning all costing in this
function (maybe digging in history shows that's what it was before?):

diff --git a/gcc/combine.cc b/gcc/combine.cc
index 053879500b7..5966da55412 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -7231,20 +7231,7 @@ expand_compound_operation (rtx x)
       && ((nonzero_bits (XEXP (x, 0), inner_mode)
           & ~(((unsigned HOST_WIDE_INT) GET_MODE_MASK (inner_mode)) >> 1))
          == 0))
-    {
-      rtx temp = gen_rtx_ZERO_EXTEND (mode, XEXP (x, 0));
-      rtx temp2 = expand_compound_operation (temp);
-
-      /* Make sure this is a profitable operation.  */
-      if (set_src_cost (x, mode, optimize_this_for_speed_p)
-          > set_src_cost (temp2, mode, optimize_this_for_speed_p))
-       return temp2;
-      else if (set_src_cost (x, mode, optimize_this_for_speed_p)
-               > set_src_cost (temp, mode, optimize_this_for_speed_p))
-       return temp;
-      else
-       return x;
-    }
+    x = gen_rtx_ZERO_EXTEND (mode, XEXP (x, 0));

   /* We can optimize some special cases of ZERO_EXTEND.  */
   if (GET_CODE (x) == ZERO_EXTEND)