[Bug rtl-optimization/99305] New: [11 Regression] range condition simplification after inlining

[Bug rtl-optimization/99305] New: [11 Regression] range condition simplification after inlining

[nok.raven at gmail dot com]

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

            Bug ID: 99305
           Summary: [11 Regression] range condition simplification after
                    inlining
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nok.raven at gmail dot com
  Target Milestone: ---

bool foo(char c)
{
    return c >= '0' && c <= '9';
}

bool bar(char c)
{
    return c != '\0' && foo(c);
}

bool foobar(char c)
{
    return c != '\0' && c >= '0' && c <= '9';
}

// GCC 10
bar(char):
  sub edi, 48
  cmp dil, 9
  setbe al
  ret

// GCC 11
bar(char):
  xor eax, eax
  test dil, dil
  je .L3
  sub edi, 48
  cmp dil, 9
  setbe al
.L3:
  ret

https://godbolt.org/z/z4r9cv

[Bug rtl-optimization/99305] [11 Regression] range condition simplification after inlining

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization,
                   |                            |needs-bisection
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Target Milestone|---                         |11.0
   Last reconfirmed|                            |2021-03-01

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  Some forwprop/match.pd prevents phiopt to trigger:

GCC 10 (forwprop->phiopt):

   <bb 2> [local count: 1073741824]:
   _7 = (unsigned char) c_2(D);
   _8 = _7 + 208;
-  if (_8 <= 9)
-    goto <bb 3>; [50.00%]
-  else
-    goto <bb 4>; [50.00%]
-
-  <bb 3> [local count: 536870913]:
-
-  <bb 4> [local count: 1073741824]:
-  # iftmp.1_1 = PHI <1(3), 0(2)>
-  return iftmp.1_1;
+  _9 = _8 <= 9;
+  return _9;

forwprop difference GCC 10/11:

-  Replaced '_9 != 0' with '_8 <= 9'
-bar (char c)
+bool bar (char c)
 {
   bool iftmp.1_1;
-  unsigned char _7;
-  unsigned char _8;
+  unsigned char c.0_4;
+  unsigned char _5;
+  bool _6;
+  bool _7;

   <bb 2> [local count: 1073741824]:
-  _7 = (unsigned char) c_2(D);
-  _8 = _7 + 208;
-  if (_8 <= 9)
+  if (c_2(D) != 0)
     goto <bb 3>; [50.00%]
   else
     goto <bb 4>; [50.00%]

   <bb 3> [local count: 536870913]:
+  c.0_4 = (unsigned char) c_2(D);
+  _5 = c.0_4 + 208;
+  _6 = _5 <= 9;
+  _7 = -_6;

   <bb 4> [local count: 1073741824]:
-  # iftmp.1_1 = PHI <1(3), 0(2)>
+  # iftmp.1_1 = PHI <_7(3), 0(2)>
   return iftmp.1_1;

[Bug tree-optimization/99305] [11 Regression] range condition simplification after inlining

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
           Keywords|needs-bisection             |

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with my r11-4717-g3e190757fa332d327bee27495f37beb01155cfab change.

[Bug tree-optimization/99305] [11 Regression] range condition simplification after inlining

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
--- gcc/tree-ssa-phiopt.c.jj    2021-01-22 11:41:38.078708425 +0100
+++ gcc/tree-ssa-phiopt.c       2021-03-09 13:15:02.649094949 +0100
@@ -808,14 +808,14 @@ conditional_replacement (basic_block con
     nonzero_arg = arg0;
   else
     return false;
-  if (integer_all_onesp (nonzero_arg))
-    neg = true;
-  else if (integer_pow2p (nonzero_arg))
+  if (integer_pow2p (nonzero_arg))
     {
       shift = tree_log2 (nonzero_arg);
       if (shift && POINTER_TYPE_P (TREE_TYPE (nonzero_arg)))
        return false;
     }
+  else if (integer_all_onesp (nonzero_arg))
+    neg = true;
   else
     return false;

should fix this I think.  For bool, integer_all_onesp and integer_onep and
integer_pow2p are all true...
We shouldn't negate in that case.

[Bug tree-optimization/99305] [11 Regression] range condition simplification after inlining

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
           Priority|P3                          |P1

[Bug tree-optimization/99305] [11 Regression] range condition simplification after inlining

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50338
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50338&action=edit
gcc11-pr99305.patch

Untested fix.

[Bug tree-optimization/99305] [11 Regression] range condition simplification after inlining

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

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

commit r11-7587-gb610c30453d8e4cc88693d85a5a100d089640be5
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Mar 9 19:13:11 2021 +0100

    phiopt: Fix up conditional_replacement [PR99305]

    Before my PR97690 changes, conditional_replacement would not set neg
    when the nonzero arg was boolean true.
    I've simplified the testing, so that it first finds the zero argument
    and then checks the other argument for all the handled cases
    (1, -1 and 1 << X, where the last case is what the patch added support
for).
    But, unfortunately I've placed the integer_all_onesp test first.
    For unsigned precision 1 types such as bool integer_all_onesp, integer_onep
    and integer_pow2p can all be true and the code set neg to true in that
case,
    which is undesirable.

    The following patch tests integer_pow2p first (which is trivially true
    for integer_onep too and tree_log2 in that case gives shift == 0)
    and only if that isn't the case, integer_all_onesp.

    2021-03-09  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/99305
            * tree-ssa-phiopt.c (conditional_replacement): Test integer_pow2p
            before integer_all_onesp instead of vice versa.

            * g++.dg/opt/pr99305.C: New test.

[Bug tree-optimization/99305] [11 Regression] range condition simplification after inlining

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed, thanks for the report.

[Bug tree-optimization/99305] [11 Regression] range condition simplification after inlining

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:5bf998275aff311b9804c1de944a88c219f35db6

commit r11-7607-g5bf998275aff311b9804c1de944a88c219f35db6
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Mar 10 17:40:25 2021 +0100

    testsuite: Fix up pr99305.C test on unsigned_char targets [PR99498]

    On unsigned_char targets, the cast stmt to unsigned char is obviously
    not needed (and shouldn't be there).  But it doesn't hurt to test
    the rest also on targets where char is unsigned.

    2021-03-10  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/99305
            PR testsuite/99498
            * g++.dg/opt/pr99305.C: Don't expect cast to unsigned char on
            unsigned_char effective targets.