[Bug tree-optimization/96674] New: Failure to optimize combination of comparisons to dec+compare

[Bug tree-optimization/96674] New: Failure to optimize combination of comparisons to dec+compare

[gabravier at gmail dot com]

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

            Bug ID: 96674
           Summary: Failure to optimize combination of comparisons to
                    dec+compare
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gabravier at gmail dot com
  Target Milestone: ---

bool f(unsigned a, unsigned b)
{
    return (b == 0) | (a < b);
}

This can be optimized to `return a <= (b - 1);`. This transformation is done by
LLVM, but not by GCC.

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-08-25

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
           Keywords|                            |easyhack

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[erozen at microsoft dot com]

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

Eugene Rozenfeld <erozen at microsoft dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |erozen at microsoft dot com

--- Comment #1 from Eugene Rozenfeld <erozen at microsoft dot com> ---
Created attachment 49940
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49940&action=edit
[PATCH] Optimize combination of comparisons to dec+compare

The patch has been approved by Richard Biener.

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[gabravier at gmail dot com]

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

--- Comment #2 from Gabriel Ravier <gabravier at gmail dot com> ---
Isn't __attribute__((noipa)) usually used instead of __attribute__((noinline))
?

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[erozen at microsoft dot com]

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

--- Comment #3 from Eugene Rozenfeld <erozen at microsoft dot com> ---
Both are used but it looks like __attribute__((noinline)) is used more
frequently.
Under gcc/testsuite there are 1537 instances of __attribute__((noipa)) and 3794
instances of __attribute__((noinline)).

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[gabravier at gmail dot com]

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

--- Comment #4 from Gabriel Ravier <gabravier at gmail dot com> ---
I'd assume those are for older test cases: __attribute__((noipa)) makes more
sense (at least to me) considering it's made specifically to prevent
inter-procedural optimization (which __attribute__((noinline)) does not, afaik)
which is helpful for testing specific optimizations on specific functions and
seeing whether the optimizations make those functions non-functional without
potentially having problems with inter-procedural optimization doing stuff like
constant-propagation or anything that could interfere with the proper testing
of a specific optimization.

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
/* x < y || x == XXX_MIN --> x <= y - 1 */
(simplify
 (bit_ior (eq @1 min_value) (lt @0 @1))
  (if (INTEGRAL_TYPE_P (TREE_TYPE (@1))
       && TYPE_OVERFLOW_WRAPS (TREE_TYPE (@1)))
  (le @0 (minus @1 { build_int_cst (TREE_TYPE (@1), 1); }))))
The comment doesn't match what the simplification implements (x == XXX_MIN
should be y == XXX_MIN).
Furthermore, bit_ior is commutative and for the optimization no specific order
is needed, so probably bit_ior:c is needed.  Also, the optimization doesn't
seem to be worth if either eq or lt has multiple uses, so both should have :s
suffixes.  When x < y || y == min can be simplified into x <= y - 1, can't
its negation, i.e. x >= y && y != min be simplified into x > y - 1 ?
And agree on the noipa attribute, most of the tests you're citing just predate
the noipa attribute.  We had noinline for many years, later added noclone and
have been using noinline, noclone and when we started adding further IPA
optimizations, noipa has been added.

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[erozen at microsoft dot com]

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

Eugene Rozenfeld <erozen at microsoft dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #49940|0                           |1
        is obsolete|                            |

--- Comment #6 from Eugene Rozenfeld <erozen at microsoft dot com> ---
Created attachment 49969
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49969&action=edit
Optimize combination of comparisons to dec+compare

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[erozen at microsoft dot com]

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

--- Comment #7 from Eugene Rozenfeld <erozen at microsoft dot com> ---
Thank you for the feedback, Gabriel and Jakub. I re-worked the patch based on
your suggestions. I attached the new patch and also sent it to gcc-patches.

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[ubizjak at gmail dot com]

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

--- Comment #8 from Uroš Bizjak <ubizjak at gmail dot com> ---
Comment on attachment 49969
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49969
Optimize combination of comparisons to dec+compare

>+/* y == XXX_MIN || x < y --> x <= y - 1 */

Can we use TYPE_MIN instead of XXX_MIN?

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[erozen at microsoft dot com]

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

--- Comment #9 from Eugene Rozenfeld <erozen at microsoft dot com> ---
I used XXX_MIN for consistency with comments on other patterns. If TYPE_MIN is
preferable, the change should be made in all of those comments as well.

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 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:49e8c14ef6f1f968602a04c8499a672182590e87

commit r11-6817-g49e8c14ef6f1f968602a04c8499a672182590e87
Author: Eugene Rozenfeld <erozen@microsoft.com>
Date:   Wed Dec 9 16:44:25 2020 -0800

    Optimize combination of comparisons to dec+compare

    This patch adds patterns for optimizing
    x < y || y == XXX_MIN to x <= y-1
    x >= y && y != XXX_MIN to x > y-1
    if y is an integer with TYPE_OVERFLOW_WRAPS.

    This fixes pr96674.

    Tested on x86_64-pc-linux-gnu.

    For this function

    bool f(unsigned a, unsigned b)
    {
        return (b == 0) | (a < b);
    }

    the code without the patch is

    test   esi,esi
    sete   al
    cmp    esi,edi
    seta   dl
    or     eax,edx
    ret

    the code with the patch is

    sub    esi,0x1
    cmp    esi,edi
    setae  al
    ret

            PR tree-optimization/96674
    gcc/
            * match.pd: New patterns: x < y || y == XXX_MIN --> x <= y - 1
            x >= y && y != XXX_MIN --> x > y - 1

    gcc/testsuite
            * gcc.dg/pr96674.c: New tests.

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[law at gcc dot gnu.org]

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

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |law at gcc dot gnu.org
         Resolution|---                         |FIXED

--- Comment #11 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Resolved by Eugene's patch on the trunk.

[Bug tree-optimization/96674] Failure to optimize combination of comparisons to dec+compare

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0