[Bug tree-optimization/103417] New: wrong code at -O1 and above on x86_64-linux-gnu

[Bug tree-optimization/103417] New: wrong code at -O1 and above on x86_64-linux-gnu

[zhendong.su at inf dot ethz.ch]

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

            Bug ID: 103417
           Summary: wrong code at -O1 and above on x86_64-linux-gnu
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zhendong.su at inf dot ethz.ch
  Target Milestone: ---

This appears to be a recent regression.

[557] % gcctk -v
Using built-in specs.
COLLECT_GCC=gcctk
COLLECT_LTO_WRAPPER=/local/suz-local/software/local/gcc-trunk/libexec/gcc/x86_64-pc-linux-gnu/12.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../gcc-trunk/configure --disable-bootstrap
--prefix=/local/suz-local/software/local/gcc-trunk --enable-languages=c,c++
--disable-werror --enable-multilib --with-system-zlib
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 12.0.0 20211124 (experimental) [master r12-5505-g5deacf6058d] (GCC) 
[558] % 
[558] % gcctk -O0 small.c; ./a.out
[559] % 
[559] % gcctk -O1 small.c
[560] % ./a.out
Aborted
[561] % 
[561] % cat small.c
struct {
  int a : 8;
  int b : 24;
} c = {0, 1};
int main() {
  if (c.b && !c.b)
    __builtin_abort();
  return 0;
}

[Bug tree-optimization/103417] wrong code at -O1 and above on x86_64-linux-gnu

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
It worked at r12-5485-ge1d4359264585

[Bug tree-optimization/103417] [12 Regression] wrong code at -O1 and above on x86_64-linux-gnu

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|unknown                     |12.0
            Summary|wrong code at -O1 and above |[12 Regression] wrong code
                   |on x86_64-linux-gnu         |at -O1 and above on
                   |                            |x86_64-linux-gnu
   Target Milestone|---                         |12.0

[Bug tree-optimization/103417] [12 Regression] wrong code at -O1 and above on x86_64-linux-gnu since r12-5489

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
                 CC|                            |jakub at gcc dot gnu.org
   Last reconfirmed|                            |2021-11-24
            Summary|[12 Regression] wrong code  |[12 Regression] wrong code
                   |at -O1 and above on         |at -O1 and above on
                   |x86_64-linux-gnu            |x86_64-linux-gnu since
                   |                            |r12-5489
           Priority|P3                          |P1

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r12-5489-g0888d6bbe97e10de0e624f4ab46acc276e5ee1d7

[Bug tree-optimization/103417] [12 Regression] wrong code at -O1 and above on x86_64-linux-gnu since r12-5489

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
generic_simplify_GE_EXPR is called with
BIT_FIELD_REF <c, 32, 0> & 4294967040U
and
0U
arguments, and transforms that into
BIT_FIELD_REF <c, 32, 0> <= 255.  That is wrong,
(BIT_FIELD_REF <c, 32, 0> & 4294967040U) >= 0U
is always true (anything >= 0U is true).

[Bug tree-optimization/103417] [12 Regression] wrong code at -O1 and above on x86_64-linux-gnu since r12-5489

[jakub at gcc dot gnu.org]

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

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 #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 51870
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51870&action=edit
gcc12-pr103417.patch

Untested fix.  Handling GE in that simplification is clearly bogus, we should
just fold it to true elsewhere, not bother with it (it doesn't handle LT
either,
which should also fold to false elsewhere).
Handling LE and GT there isn't wrong, but makes no sense.  Elsewhere we
canonicalize x > 0U into x != 0U and x <= 0U into x == 0U and for signed it
was handling only EQ and NE already before.

[Bug tree-optimization/103417] [12 Regression] wrong code at -O1 and above on x86_64-linux-gnu since r12-5489

[tnfchris at gcc dot gnu.org]

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

--- Comment #5 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #4)
> Created attachment 51870 [details]
> gcc12-pr103417.patch
> 
> Untested fix.  Handling GE in that simplification is clearly bogus, we
> should just fold it to true elsewhere, not bother with it (it doesn't handle
> LT either,
> which should also fold to false elsewhere).

Indeed, that one is wrong..

> Handling LE and GT there isn't wrong, but makes no sense.  Elsewhere we
> canonicalize x > 0U into x != 0U and x <= 0U into x == 0U and for signed it
> was handling only EQ and NE already before.

Well, the intention is to simplify the bitmask. Most vector ISAs can create the
simple bitmask much easier than the complex one. i.e. 0xFFFFFF00 is much harder
to create than 0xFF. for scalar yes it doesn't matter much.

but e.g.

     for (int i = 0; i < (n & -16); i++)
       x[i] = (x[i]&(~255)) <= 0U;

generates worse code when the mask 0xFFFFFF00 is to be used.  The patch is
mainly addressing vector code but we added scalar for uniformity.

So I would like LE and GT to stay, at the very least for vector where it makes
a difference.  It's not something we can fix in the backend because we can't
differentiate between signed and unsigned.

[Bug tree-optimization/103417] [12 Regression] wrong code at -O1 and above on x86_64-linux-gnu since r12-5489

[cvs-commit at gcc dot gnu.org]

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

--- 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:94912212d3d1be0b1c490e9b5f45165ef5f30d8a

commit r12-5513-g94912212d3d1be0b1c490e9b5f45165ef5f30d8a
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Nov 25 10:47:24 2021 +0100

    match.pd: Fix up the recent bitmask_inv_cst_vector_p simplification
[PR103417]

    The following testcase is miscompiled since the r12-5489-g0888d6bbe97e10
    changes.
    The simplification triggers on
    (x & 4294967040U) >= 0U
    and turns it into:
    x <= 255U
    which is incorrect, it should fold to 1 because unsigned >= 0U is always
    true and normally the
    /* Non-equality compare simplifications from fold_binary  */
         (if (wi::to_wide (cst) == min)
           (if (cmp == GE_EXPR)
            { constant_boolean_node (true, type); })
    simplification folds that, but this simplification was done earlier.

    The simplification correctly doesn't include lt which has the same
    reason why it shouldn't be handled, we'll fold it to 0 elsewhere.

    But, IMNSHO while it isn't incorrect to handle le and gt there, it is
    unnecessary.  Because (x & cst) <= 0U and (x & cst) > 0U should
    never appear, again in
    /* Non-equality compare simplifications from fold_binary  */
    we have a simplification for it:
           (if (cmp == LE_EXPR)
            (eq @2 @1))
           (if (cmp == GT_EXPR)
            (ne @2 @1))))
    This is done for
      (cmp (convert?@2 @0) uniform_integer_cst_p@1)
    and so should be done for both integers and vectors.
    As the bitmask_inv_cst_vector_p simplification only handles
    eq and ne for signed types, I think it can be simplified to just
    following patch.

    2021-11-25  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/103417
            * match.pd ((X & Y) CMP 0): Only handle eq and ne.  Commonalize
            common tests.

            * gcc.c-torture/execute/pr103417.c: New test.

[Bug tree-optimization/103417] [12 Regression] wrong code at -O1 and above on x86_64-linux-gnu since r12-5489

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.