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

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

[zhendong.su at inf dot ethz.ch]

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

            Bug ID: 103376
           Summary: wrong code at -Os 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: ---

It appears to be a recent regression.

[603] % 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 20211123 (experimental) [master r12-5461-gcab8f69857d] (GCC) 
[604] % 
[604] % gcctk -O1 small.c; ./a.out
[605] % 
[605] % gcctk -Os small.c
[606] % ./a.out
Aborted
[607] % 
[607] % cat small.c
int printf(const char *, ...);
long long a = 7108473123;
int b, c, *d;
int main() {
  long long e, f;
  e = a;
  if (b) {
    printf("%d", c);
    d = 0;
    while (*d)
      ;
  }
  f = 0x88582324C7C1E17B;
  a = ~-(e ^ f ^ ~a) ^ ~18013;
  if (a > -8622102845723662559LL)
    __builtin_abort();
}

[Bug tree-optimization/103376] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5453-ga944b5dec3adb28e

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.0
           Priority|P3                          |P1
             Status|UNCONFIRMED                 |NEW
            Summary|wrong code at -Os and above |[12 Regression] wrong code
                   |on x86_64-linux-gnu         |at -Os and above on
                   |                            |x86_64-linux-gnu since
                   |                            |r12-5453-ga944b5dec3adb28e
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-11-23
                 CC|                            |marxin at gcc dot gnu.org,
                   |                            |roger at nextmovesoftware dot com

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r12-5453-ga944b5dec3adb28e.

[Bug tree-optimization/103376] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5453-ga944b5dec3adb28e

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, we have:
  e_11 = a;
...
  a.3_5 = a;
  _17 = a.3_5 ^ e_11;
and no stores in between those 2 reads, initially there has been a bb that
could fall through and that is the reason why the two loads are still there,
but it has been changed a few passes back to end with __builtin_trap.
bswap pass changes this to:
  e_11 = a;
...
+  load_dst_9 = MEM[(long long int *)&a];
+  _17 = (long long int) load_dst_9;
   a.3_5 = a;
-  _17 = a.3_5 ^ e_11;
For | instead of ^ that is a correct optimization a | a is still a, but for the
newly added operations a ^ a is 0 rather than a and a + a is usually different
than a too.
So, I guess for ^ and + we need to perform extra checking.
perform_symbolic_merge is doing:
  for (i = 0, mask = MARKER_MASK; i < size; i++, mask <<= BITS_PER_MARKER)
    {
      uint64_t masked1, masked2;

      masked1 = n1->n & mask;
      masked2 = n2->n & mask;
      if (masked1 && masked2 && masked1 != masked2)
        return NULL;
    }
which is correct solely for bitwise or.  For other operations it would need to
do instead if (masked1 && masked2) return NULL; instead.

[Bug tree-optimization/103376] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5453-ga944b5dec3adb28e

[jakub at gcc dot gnu.org]

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

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

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

[Bug tree-optimization/103376] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5453-ga944b5dec3adb28e

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 51858
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51858&action=edit
gcc12-pr103376.patch

Untested fix.

[Bug tree-optimization/103376] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5453-ga944b5dec3adb28e

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zsojka at seznam dot cz

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
*** Bug 103380 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/103376] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5453-ga944b5dec3adb28e

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vsevolod.livinskij at frtk dot ru

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 103399 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/103376] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5453-ga944b5dec3adb28e

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Another testcase (from PR 103399) which shows the issue:
int t1 = -2;
int e(int) __attribute__((noipa));
int e(int f) {
    int t = 0;
  for (int d = 0; d < f; d ++) {
    for (int c = 0; c < ((f ? t1 : 0) ^ t1) + 1; c++)
      t= 42;
  }
 return t;
}
int main() {
  unsigned a = e(1);
  printf("%u\n", a);
  if (a != 42)
    __builtin_abort ();
}

[Bug tree-optimization/103376] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5453-ga944b5dec3adb28e

[zhendong.su at inf dot ethz.ch]

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

--- Comment #7 from Zhendong Su <zhendong.su at inf dot ethz.ch> ---
A couple of additional tests for the same issue:

(1) at -Os and above

[622] % gcctk -O1 small.c; ./a.out
[623] % gcctk -Os small.c; ./a.out
Aborted
[624] % cat small.c
long a = 2653121401;
unsigned char b;
int c;
int main() {
  b = a;
  c = ~(((a ^ b) + 2) ^ 254);
  if (c > 1641845765)
    __builtin_abort();
  return 0;
}

(2) at -O2 and -O3

[511] % gcctk -Os small.c; ./a.out
[512] % gcctk -O2 small.c; ./a.out
Segmentation fault
[513] % 
[513] % cat small.c
int printf(const char *, ...);
int a = 1, *b;
int main() {
  int c = a;
  if (a == 2)
    printf("0");
  a = a ^ c;
  if (a)
    a++;
  if (a && *b)
    return a;
  return 0;
}

[Bug tree-optimization/103376] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5453-ga944b5dec3adb28e

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 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:04eccbbe3d9a4e9d2f8f43dba8ac4cb686029fb2

commit r12-5492-g04eccbbe3d9a4e9d2f8f43dba8ac4cb686029fb2
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Nov 24 09:54:44 2021 +0100

    bswap: Fix up symbolic merging for xor and plus [PR103376]

    On Mon, Nov 22, 2021 at 08:39:42AM -0000, Roger Sayle wrote:
    > This patch implements PR tree-optimization/103345 to merge adjacent
    > loads when combined with addition or bitwise xor.  The current code
    > in gimple-ssa-store-merging.c's find_bswap_or_nop alreay handles ior,
    > so that all that's required is to treat PLUS_EXPR and BIT_XOR_EXPR in
    > the same way at BIT_IOR_EXPR.

    Unfortunately they aren't exactly the same.  They work the same if always
    at least one operand (or corresponding byte in it) is known to be 0,
    0 | 0 = 0 ^ 0 = 0 + 0 = 0.  But for | also x | x = x for any other x,
    so perform_symbolic_merge has been accepting either that at least one
    of the bytes is 0 or that both are the same, but that is wrong for ^
    and +.

    The following patch fixes that by passing through the code of binary
    operation and allowing non-zero masked1 == masked2 through only
    for BIT_IOR_EXPR.

    Thinking more about it, perhaps we could do more for BIT_XOR_EXPR.
    We could allow masked1 == masked2 case for it, but would need to
    do something different than the
      n->n = n1->n | n2->n;
    we do on all the bytes together.
    In particular, for masked1 == masked2 if masked1 != 0 (well, for 0
    both variants are the same) and masked1 != 0xff we would need to
    clear corresponding n->n byte instead of setting it to the input
    as x ^ x = 0 (but if we don't know what x and y are, the result is
    also don't know).  Now, for plus it is much harder, because not only
    for non-zero operands we don't know what the result is, but it can
    modify upper bytes as well.  So perhaps only if current's byte
    masked1 && masked2 set the resulting byte to 0xff (unknown) iff
    the byte above it is 0 and 0, and set that resulting byte to 0xff too.
    Also, even for | we could instead of return NULL just set the resulting
    byte to 0xff if it is different, perhaps it will be masked off later on.

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

            PR tree-optimization/103376
            * gimple-ssa-store-merging.c (perform_symbolic_merge): Add CODE
            argument.  If CODE is not BIT_IOR_EXPR, ensure that one of masked1
            or masked2 is 0.
            (find_bswap_or_nop_1, find_bswap_or_nop,
            imm_store_chain_info::try_coalesce_bswap): Adjust
            perform_symbolic_merge callers.

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

[Bug tree-optimization/103376] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5453-ga944b5dec3adb28e

[jakub at gcc dot gnu.org]

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

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

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

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

[Bug tree-optimization/103376] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5453-ga944b5dec3adb28e

[roger at nextmovesoftware dot com]

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

--- Comment #10 from Roger Sayle <roger at nextmovesoftware dot com> ---
Many thanks to Jakub for the speedy fix (and Richard B for the speedy review),
and my apologies for the inconvenience.  As diagnosed by Jakub, I hadn't
realized that the bswap pass was performing additional optimizations, and
simplifying x op x as just x, doesn't work for plus and xor the same way as it
does for ior.  Thank you to Jakub again.

[Bug tree-optimization/103376] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5453-ga944b5dec3adb28e

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 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:531dae29a67e915a145d908bd2f46d22bc369c11

commit r12-5512-g531dae29a67e915a145d908bd2f46d22bc369c11
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Nov 25 10:38:33 2021 +0100

    bswap: Improve perform_symbolic_merge [PR103376]

    Thinking more about it, perhaps we could do more for BIT_XOR_EXPR.
    We could allow masked1 == masked2 case for it, but would need to
    do something different than the
      n->n = n1->n | n2->n;
    we do on all the bytes together.
    In particular, for masked1 == masked2 if masked1 != 0 (well, for 0
    both variants are the same) and masked1 != 0xff we would need to
    clear corresponding n->n byte instead of setting it to the input
    as x ^ x = 0 (but if we don't know what x and y are, the result is
    also don't know).  Now, for plus it is much harder, because not only
    for non-zero operands we don't know what the result is, but it can
    modify upper bytes as well.  So perhaps only if current's byte
    masked1 && masked2 set the resulting byte to 0xff (unknown) iff
    the byte above it is 0 and 0, and set that resulting byte to 0xff too.
    Also, even for | we could instead of return NULL just set the resulting
    byte to 0xff if it is different, perhaps it will be masked off later on.

    This patch just punts on plus if both corresponding bytes are non-zero,
    otherwise implements the above.

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

            PR tree-optimization/103376
            * gimple-ssa-store-merging.c (perform_symbolic_merge): For
            BIT_IOR_EXPR, if masked1 && masked2 && masked1 != masked2, don't
            punt, but set the corresponding result byte to MARKER_BYTE_UNKNOWN.
            For BIT_XOR_EXPR similarly and if masked1 == masked2 and the
            byte isn't MARKER_BYTE_UNKNOWN, set the corresponding result byte
to
            0.

            * gcc.dg/optimize-bswapsi-7.c: New test.

[Bug tree-optimization/103376] [12 Regression] wrong code at -Os and above on x86_64-linux-gnu since r12-5453-ga944b5dec3adb28e

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jan Hubicka <hubicka@gcc.gnu.org>:

https://gcc.gnu.org/g:4943b75e9f06f0b64ed541430bb7fbccf55fc552

commit r12-8070-g4943b75e9f06f0b64ed541430bb7fbccf55fc552
Author: Jan Hubicka <jh@suse.cz>
Date:   Sat Apr 9 21:22:58 2022 +0200

    Update semantic_interposition flag at analysis time

    This patch solves problem with FE first finalizing function and then adding
    -fno-semantic-interposition flag (by parsing optimization attribute).

    gcc/ChangeLog:

    2022-04-09  Jan Hubicka  <hubicka@ucw.cz>

            PR ipa/103376
            * cgraphunit.cc (cgraph_node::analyze): update
semantic_interposition
            flag.

    gcc/testsuite/ChangeLog:

    2022-04-09  Jan Hubicka  <hubicka@ucw.cz>

            PR ipa/103376
            * gcc.c-torture/compile/pr103376.c: New test.