[Bug middle-end/105629] New: [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x

[Bug middle-end/105629] New: [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x

[hp at gcc dot gnu.org]

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

            Bug ID: 105629
           Summary: [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k,
                    s390x
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hp at gcc dot gnu.org
  Target Milestone: ---

Created attachment 52987
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52987&action=edit
pr94589-2.C.252t.optimized

The test g++.dg/opt/pr94589-2.C is last known passing for cris-elf with
r13-172-g1e2334302d6ad5.  For r13-211-g0c7bce0ac184c0 it failed, and has failed
since then, including r13-555-g1815462a6e53:

testrun output:
FAIL: g++.dg/opt/pr94589-2.C  -std=gnu++20  scan-tree-dump-times optimized
"[ij]_[0-9]+\\(D\\) (?:<|<=|==|!=|>|>=) [ij]_[0-9]+\\(D\\)" 12
FAIL: g++.dg/opt/pr94589-2.C  -std=gnu++20  scan-tree-dump-times optimized
"i_[0-9]+\\(D\\) (?:<|<=|==|!=|>|>=) 5\\.0" 12

In g++.log:
PASS: g++.dg/opt/pr94589-2.C  -std=gnu++20 (test for excess errors)
g++.dg/opt/pr94589-2.C  -std=gnu++20 : pattern found 14 times
FAIL: g++.dg/opt/pr94589-2.C  -std=gnu++20  scan-tree-dump-times optimized
"[ij]_[0-9]+\\(D\\) (?:<|<=|==|!=|>|>=) [ij]_[0-9]+\\(D\\)" 12
g++.dg/opt/pr94589-2.C  -std=gnu++20 : pattern found 14 times
FAIL: g++.dg/opt/pr94589-2.C  -std=gnu++20  scan-tree-dump-times optimized
"i_[0-9]+\\(D\\) (?:<|<=|==|!=|>|>=) 5\\.0" 12

See attachment for contents of pr94589-2.C.252t.optimized at
r13-555-g1815462a6e53.

The test appears to fail in the same way according to gcc-testresults@, for
m68k-unknown-linux-gnu
(posts https://gcc.gnu.org/pipermail/gcc-testresults/2022-May/761149.html and
https://gcc.gnu.org/pipermail/gcc-testresults/2022-May/761602.html) and
s390x-ibm-linux-gnu (posts
https://gcc.gnu.org/pipermail/gcc-testresults/2022-May/761118.html and
https://gcc.gnu.org/pipermail/gcc-testresults/2022-May/761552.html).

[Bug middle-end/105629] [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0

[Bug middle-end/105629] [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x

[hp at gcc dot gnu.org]

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

Hans-Peter Nilsson <hp at gcc dot gnu.org> changed:

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

--- Comment #1 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
It's r13-204-g93416de0cb7235, "Remove GIMPLE restriction of ! using match.pd
patterns".  A brief look at the diff of the assembly between that and that^
indicates a code quality regression.

[Bug middle-end/105629] [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x

[rguenther at suse dot de]

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

--- Comment #2 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 19 May 2022, hp at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105629
> 
> Hans-Peter Nilsson <hp at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |rguenth at gcc dot gnu.org
> 
> --- Comment #1 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
> It's r13-204-g93416de0cb7235, "Remove GIMPLE restriction of ! using match.pd
> patterns".  A brief look at the diff of the assembly between that and that^
> indicates a code quality regression.

I wonder if you can help and "bisect" the actual pattern that is now
enabled for GENERIC folding?  The testcase is about spaceship optimization
and the matching done in forwprop might be quite specific and in the need
of a slight adjustment.

[Bug middle-end/105629] [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x

[hp at gcc dot gnu.org]

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

--- Comment #3 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #2)
> On Thu, 19 May 2022, hp at gcc dot gnu.org wrote:
> I wonder if you can help and "bisect" the actual pattern that is now
> enabled for GENERIC folding?  The testcase is about spaceship optimization
> and the matching done in forwprop might be quite specific and in the need
> of a slight adjustment.

I'm going to interpret that as "if each pattern had its own #if
GIMPLE-wrapping, which hunk in the diff caused the regression".  I'll try and
see if it's as simple as it sounds.

[Bug middle-end/105629] [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x

[rguenther at suse dot de]

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

--- Comment #4 from rguenther at suse dot de <rguenther at suse dot de> ---
On Fri, 20 May 2022, hp at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105629
> 
> --- Comment #3 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
> (In reply to rguenther@suse.de from comment #2)
> > On Thu, 19 May 2022, hp at gcc dot gnu.org wrote:
> > I wonder if you can help and "bisect" the actual pattern that is now
> > enabled for GENERIC folding?  The testcase is about spaceship optimization
> > and the matching done in forwprop might be quite specific and in the need
> > of a slight adjustment.
> 
> I'm going to interpret that as "if each pattern had its own #if
> GIMPLE-wrapping, which hunk in the diff caused the regression".  I'll try and
> see if it's as simple as it sounds.

Yes - thanks.

[Bug middle-end/105629] [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x

[hp at gcc dot gnu.org]

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

--- Comment #5 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
Additional observation: the diff is local to functions f5 and f17 (in
g++.dg/opt/pr94589-2.C), i.e. "reduced" to (for reference only):

A bool f5 (double i, double j) { auto c = i <=> j; return c >= 0; }
A bool f17 (double i) { auto c = i <=> 5.0; return c >= 0; }

[Bug middle-end/105629] [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x

[hp at gcc dot gnu.org]

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

--- Comment #6 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
Created attachment 53011
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53011&action=edit
Reduced fix on top of 93416de0cb; a fragmental revert

The attached patch, applied to 93416de0cb, re-introduces the one of the removed
#if GIMPLE clauses, but narrowed to the one (simplify ...) clause, that
mattered for the regression.  (Not regtested though, just pr94589-2.C
re-compiled.)

I have no idea how this pattern matched here, as there appears to be no
"bit_and" expressions in the affected functions.  I'd suspect some kind of
saddle-point/local-minimum false lead, except then there'd reasonably be some
half-way transformation left in the code.  But then again, reasoning around
bugs you don't fully understand is rarely useful.

Anyway, I hope this helped.

[Bug middle-end/105629] [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
           Priority|P3                          |P1
   Last reconfirmed|                            |2022-05-23
             Target|                            |x86_64-*-*
     Ever confirmed|0                           |1
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Thanks, so the relevant IL change is probably

  # c$_M_value_3 = PHI <-1(3), 0(2), 2(5), 1(4)>
  _11 = (unsigned char) c$_M_value_3;
  _16 = _11 <= 1;

vs

  # c$_M_value_3 = PHI <-1(3), 0(2), 2(5), 1(4)>
  _11 = (unsigned int) c$_M_value_3;
  _16 = _11 <= 1;

where the conversion from c$_M_value_3 is now sign-extending which is something
spaceship_replacement does not expect.  We how do

Matching expression match.pd:2114, generic-match.cc:676
Matching expression match.pd:2121, generic-match.cc:736
Applying pattern match.pd:5497, generic-match.cc:23783   (****)
Matching expression match.pd:2114, generic-match.cc:676
Matching expression match.pd:2121, generic-match.cc:736
Matching expression match.pd:2126, generic-match.cc:776
Applying pattern match.pd:5868, generic-match.cc:62355
Applying pattern match.pd:3662, generic-match.cc:27474
Applying pattern match.pd:3742, generic-match.cc:26853
Applying pattern match.pd:3648, generic-match.cc:27432

;; Function constexpr bool std::operator>=(partial_ordering,
__cmp_cat::__unspec) (null)
;; enabled by -tree-original


<<< Unknown tree: must_not_throw_expr
  return <retval> = (unsigned int) __v._M_value <= 1
   >>>;

with the marked (****) folding that eventually you re-disable for GENERIC.

In the end we use .SPACESHIP on x86_64 but end up with

bool f17 (double i)
{
  bool _2;
  int _7;
  bool prephitmp_8;

  <bb 2> [local count: 1073741824]:
  _7 = .SPACESHIP (i_1(D), 5.0e+0);
  if (_7 != 0)
    goto <bb 3>; [50.00%]
  else
    goto <bb 5>; [50.00%]

  <bb 3> [local count: 536870913]:
  if (_7 == 1)
    goto <bb 4>; [50.00%]
  else
    goto <bb 5>; [50.00%]

  <bb 4> [local count: 268435456]:
  _2 = i_1(D) > 5.0e+0;

  <bb 5> [local count: 1073741824]:
  # prephitmp_8 = PHI <1(2), _2(4), 0(3)>
  return prephitmp_8;

which is a regression from GCC 12.  I think the non-widening constraint can
be relaxed.  Indeed the following fixes it - Jakub, is there anything
that would prevent sign-extending to work?

diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index 8c9c46d41f1..e61d9736937 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -2217,7 +2217,7 @@ spaceship_replacement (basic_block cond_bb, basic_block
middle_bb,

       if (!TYPE_UNSIGNED (ty2) || !INTEGRAL_TYPE_P (ty2))
        return false;
-      if (TYPE_PRECISION (ty1) != TYPE_PRECISION (ty2))
+      if (TYPE_PRECISION (ty1) > TYPE_PRECISION (ty2))
        return false;
       if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (orig_use_lhs))
        return false;

[Bug middle-end/105629] [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:d918faea1217596877a35c4946500399731fbbd3

commit r13-722-gd918faea1217596877a35c4946500399731fbbd3
Author: Richard Biener <rguenther@suse.de>
Date:   Mon May 23 11:41:50 2022 +0200

    tree-optimization/105629 - spaceship recognition regression

    With the extra GENERIC folding we now do to
    (unsigned int) __v._M_value & 1 != (unsigned int) __v._M_value
    we end up with a sign-extending conversion to unsigned int
    rather than the sign-conversion to unsigned char we expect.
    Relaxing that fixes the regression.

    2022-05-23  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/105629
            * tree-ssa-phiopt.cc (spaceship_replacement): Allow
            a sign-extending conversion.

[Bug middle-end/105629] [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Shoudl be fixed.

[Bug middle-end/105629] [13 Regression] g++.dg/opt/pr94589-2.C for cris, m68k, s390x

[hp at gcc dot gnu.org]

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

--- Comment #10 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #9)
> Shoudl be fixed.

Confirmed, thanks!