[Bug tree-optimization/105776] New: Failure to recognize __builtin_mul_overflow pattern

[Bug tree-optimization/105776] New: Failure to recognize __builtin_mul_overflow pattern

[gabravier at gmail dot com]

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

            Bug ID: 105776
           Summary: Failure to recognize __builtin_mul_overflow pattern
           Product: gcc
           Version: 13.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: ---

int f4(unsigned x, unsigned y)
{
    if (x == 0)
        return 1;
    return ((int)(x * y) / (int)x) == y;
}

can be optimized to

int f4(unsigned x, unsigned y)
{
    int z;
    return !__builtin_mul_overflow((int)x, (int)y, &z);
}

This transformation is done by LLVM, but not by GCC.

Note that this derivates from another function written as such:

int
f3 (unsigned x, unsigned y)
{
  unsigned int r = x * y;
  return !x || ((int) r / (int) x) == (int) y;
}

which does optimize correctly on x86 but not on aarch64 (where it generates
tree-optimized GIMPLE corresponding to the code above)

[Bug tree-optimization/105776] Failure to recognize __builtin_mul_overflow pattern

[pinskia at gcc dot gnu.org]

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

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

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

[Bug tree-optimization/105776] Failure to recognize __builtin_mul_overflow pattern

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note there are two different issues here.





The second one is a target issue.
For f3, widening multiple pass (which is misnamed these days) detects the
__builtin_mul_overflow for x86_64 but not for aarch64 even though the incoming
IR is the same.

[Bug tree-optimization/105776] Failure to recognize __builtin_mul_overflow pattern

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #1)
> The second one is a target issue.
> For f3, widening multiple pass (which is misnamed these days) detects the
> __builtin_mul_overflow for x86_64 but not for aarch64 even though the
> incoming IR is the same.

I will file it seperately, the problem there is umulv4/mulv4 patterns are not
defined for aarch64.

[Bug tree-optimization/105776] Failure to recognize __builtin_mul_overflow pattern

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=101856

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #2)
> (In reply to Andrew Pinski from comment #1)
> > The second one is a target issue.
> > For f3, widening multiple pass (which is misnamed these days) detects the
> > __builtin_mul_overflow for x86_64 but not for aarch64 even though the
> > incoming IR is the same.
> 
> I will file it seperately, the problem there is umulv4/mulv4 patterns are
> not defined for aarch64.

Actually I already filed it as PR 101856 :).

[Bug tree-optimization/105776] Failure to recognize __builtin_mul_overflow pattern

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2023-05-17

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The difference between f3 and f4 for the IR:

  y.3_4 = (int) y_7(D);
  _10 = _3 == y.3_4;

vs
  _5 = (unsigned int) _4;
  _6 = _5 == y_9(D);

The pass must be only ready for f3 comparison rather than the one in f4.
Both are the same really, casting to `unsigned int` one side rather than the
other side casting to `int` is both valid thing to do.

[Bug tree-optimization/105776] Failure to recognize __builtin_mul_overflow pattern

[jakub at gcc dot gnu.org]

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

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 55112
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55112&action=edit
gcc14-pr105776.patch

Untested fix.

[Bug tree-optimization/105776] Failure to recognize __builtin_mul_overflow pattern

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 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:bd0f2828432918a16e93d9e9021a5927143b8dde

commit r14-993-gbd0f2828432918a16e93d9e9021a5927143b8dde
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri May 19 12:58:32 2023 +0200

    tree-ssa-math-opts: Pattern recognize some further hand written forms of
signed __builtin_mul_overflow{,_p} [PR105776]

    In the pattern recognition of signed __builtin_mul_overflow{,_p} we
    check for result of unsigned division (which follows unsigned
    multiplication) being equality compared against one of the multiplication's
    argument (the one not used in the division) and check for the comparison
    to be done against same precision cast of the argument (because
    division's result is unsigned and the argument is signed).
    But as shown in this PR, one can write it equally as comparison done in
    the signed type, i.e. compare division's result cast to corresponding
    signed type against the argument.

    The following patch handles even those cases.

    2023-05-19  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/105776
            * tree-ssa-math-opts.cc (arith_overflow_check_p): If cast_stmt is
            non-NULL, allow division statement to have a cast as single imm use
            rather than comparison/condition.
            (match_arith_overflow): In that case remove the cast stmt in
addition
            to the division statement.

            * gcc.target/i386/pr105776.c: New test.

[Bug tree-optimization/105776] Failure to recognize __builtin_mul_overflow pattern

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Should be fixed now.