[Bug c/97073] New: Miscompilation with -m32 -O1 -march=i686

[Bug c/97073] New: Miscompilation with -m32 -O1 -march=i686

[sirl at gcc dot gnu.org]

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

            Bug ID: 97073
           Summary: Miscompilation with -m32 -O1 -march=i686
           Product: gcc
           Version: 8.4.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sirl at gcc dot gnu.org
  Target Milestone: ---

Created attachment 49229
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49229&action=edit
Testcase demonstrating the problem.

Hi,

the attached simple testcase aborts when compiled with every version since
gcc-7 (last tested r11-3232).
Compiled with gcc-6.3.1 the testcase doesn't abort.
Also adding either -fno-tree-ter or -msse2 prevents the abort.

[Bug target/97073] [8/9/10/11 Regression] Miscompilation with -m32 -O1 -march=i686

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-09-17
     Ever confirmed|0                           |1
           Priority|P3                          |P2
      Known to work|                            |6.3.1
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |8.5
            Summary|Miscompilation with -m32    |[8/9/10/11 Regression]
                   |-O1 -march=i686             |Miscompilation with -m32
                   |                            |-O1 -march=i686
      Known to fail|                            |10.2.1

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed on the GCC 10 branch.

[Bug middle-end/97073] [8/9/10/11 Regression] Miscompilation with -m32 -O1 -march=i686

[ubizjak at gmail dot com]

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

Uroš Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|target                      |middle-end

--- Comment #2 from Uroš Bizjak <ubizjak at gmail dot com> ---
Middle-end expands double mode operation to:

    2: r85:DI=[r76:SI]
    3: NOTE_INSN_FUNCTION_BEG
    6: r86:SI=`gs2'
    7: r87:SI=`gs2'
   10: {r89:SI=r85:DI#0&[r87:SI];clobber flags:CC;}
   11: [r86:SI+0x4]=r89:SI
   14: {r91:SI=r85:DI#4&[r87:SI+0x4];clobber flags:CC;}
   15: [r86:SI+0x8]=r91:SI

Please note how (insn 11) partially clobbers input value, needed by (insn 14).

When -msse2 is used, we expand with:

    6: r86:SI=`gs2'
    7: r87:SI=`gs2'
   10: {r89:DI=r85:DI&[r87:SI];clobber flags:CC;}
   11: [r86:SI+0x4]=r89:DI

that bypasses generic middle-end expansion.

[Bug middle-end/97073] [8/9/10/11 Regression] Miscompilation with -m32 -O1 -march=i686

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Shorter testcase:

typedef unsigned long long L;
union U { L i; struct T { unsigned k; L l; } j; } u;

__attribute__((noinline,noclone)) void
foo (L x)
{
  u.j.l = u.i & x;
}

int
main ()
{
  u.i = 5;
  foo (-1ULL);
  if (u.j.l != 5)
    __builtin_abort ();
  return 0;
}

This broke in between r102000 (still good) and r104000 (already miscompiled),
so I don't believe that 6.3.1 worked.

[Bug middle-end/97073] [8/9/10/11 Regression] Miscompilation with -m32 -O1 -march=i686

[jakub at gcc dot gnu.org]

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

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

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

Untested fix.

[Bug middle-end/97073] [8/9/10/11 Regression] Miscompilation with -m32 -O1 -march=i686

[sirl at gcc dot gnu.org]

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

--- Comment #5 from Franz Sirl <sirl at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> This broke in between r102000 (still good) and r104000 (already
> miscompiled), so I don't believe that 6.3.1 worked.

Hmm, maybe something in 6.3.1 is masking the bug? Yesterday I used "gcc version
6.3.1 20170216 (Red Hat 6.3.1-3) (GCC)" from
devtoolset-6-gcc-6.3.1-3.1.el7.x86_64 to try.

But in the meantime I built 6.5.0 myself and surely enough it shows the bug
too?

[Bug middle-end/97073] [8/9/10/11 Regression] Miscompilation with -m32 -O1 -march=i686

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The only explanation would be that -msse2 -mstv was enabled...  In that case
the bitwise logical op is done through pandn instruction.

[Bug middle-end/97073] [8/9/10/11 Regression] Miscompilation with -m32 -O1 -march=i686

[sirl at gcc dot gnu.org]

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

Franz Sirl <sirl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|6.3.1                       |

--- Comment #7 from Franz Sirl <sirl at gcc dot gnu.org> ---
No, my fault. At that machine I still used a testcase much closer to the
original code and that one passed. With the testcase in the bugreport it also
aborts.
Sorry for not re-checking!

[Bug middle-end/97073] [8/9/10/11 Regression] Miscompilation with -m32 -O1 -march=i686

[sirl at gcc dot gnu.org]

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

--- Comment #8 from Franz Sirl <sirl at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #4)
> Created attachment 49236 [details]
> gcc11-pr97073.patch
> 
> Untested fix.

I can confirm that this patch applied to the gcc-8 branch fixes the testcase
and the original problem in the affected application.

[Bug middle-end/97073] [8/9/10/11 Regression] Miscompilation with -m32 -O1 -march=i686

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 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:a4b31d5807f2bc67c8999b3d53369cf2a5c6e1ec

commit r11-3489-ga4b31d5807f2bc67c8999b3d53369cf2a5c6e1ec
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sun Sep 27 23:18:26 2020 +0200

    optabs: Don't reuse target for multi-word expansions if it overlaps
operand(s) [PR97073]

    The following testcase is miscompiled on i686-linux, because
    we try to expand a double-word bitwise logic operation with op0
    being a (mem:DI u) and target (mem:DI u+4), i.e. partial overlap, and
    thus end up with:
            movl    4(%esp), %eax
            andl    u, %eax
            movl    %eax, u+4
    ! movl u+4, %eax optimized out
            andl    8(%esp), %eax
            movl    %eax, u+8
    rather than with the desired:
            movl    4(%esp), %edx
            movl    8(%esp), %eax
            andl    u, %edx
            andl    u+4, %eax
            movl    %eax, u+8
            movl    %edx, u+4
    because the store of the first word to target overwrites the second word of
    the operand.
    expand_binop for this (and several similar places) already check for target
    == op0 or target == op1, this patch just adds reg_overlap_mentioned_p calls
    next to it.
    Pedantically, at least for some of these it might be sufficient to force
    a different target if there is overlap but target is not rtx_equal_p to
    the operand (e.g. in this bitwise logical case, but e.g. not in the shift
    cases where there is reordering), though that would go against the
    preexisting target == op? checks and the rationale that REG_EQUAL notes in
    that case isn't correct.

    2020-09-27  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/97073
            * optabs.c (expand_binop, expand_absneg_bit, expand_unop,
            expand_copysign_bit): Check reg_overlap_mentioned_p between target
            and operand(s) and if it returns true, force a pseudo as target.

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

[Bug middle-end/97073] [8/9/10/11 Regression] Miscompilation with -m32 -O1 -march=i686

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:9389e3abc1fc4881f22c7376aae2dd650af2b792

commit r10-8804-g9389e3abc1fc4881f22c7376aae2dd650af2b792
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sun Sep 27 23:18:26 2020 +0200

    optabs: Don't reuse target for multi-word expansions if it overlaps
operand(s) [PR97073]

    The following testcase is miscompiled on i686-linux, because
    we try to expand a double-word bitwise logic operation with op0
    being a (mem:DI u) and target (mem:DI u+4), i.e. partial overlap, and
    thus end up with:
            movl    4(%esp), %eax
            andl    u, %eax
            movl    %eax, u+4
    ! movl u+4, %eax optimized out
            andl    8(%esp), %eax
            movl    %eax, u+8
    rather than with the desired:
            movl    4(%esp), %edx
            movl    8(%esp), %eax
            andl    u, %edx
            andl    u+4, %eax
            movl    %eax, u+8
            movl    %edx, u+4
    because the store of the first word to target overwrites the second word of
    the operand.
    expand_binop for this (and several similar places) already check for target
    == op0 or target == op1, this patch just adds reg_overlap_mentioned_p calls
    next to it.
    Pedantically, at least for some of these it might be sufficient to force
    a different target if there is overlap but target is not rtx_equal_p to
    the operand (e.g. in this bitwise logical case, but e.g. not in the shift
    cases where there is reordering), though that would go against the
    preexisting target == op? checks and the rationale that REG_EQUAL notes in
    that case isn't correct.

    2020-09-27  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/97073
            * optabs.c (expand_binop, expand_absneg_bit, expand_unop,
            expand_copysign_bit): Check reg_overlap_mentioned_p between target
            and operand(s) and if it returns true, force a pseudo as target.

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

    (cherry picked from commit a4b31d5807f2bc67c8999b3d53369cf2a5c6e1ec)

[Bug middle-end/97073] [8/9/10/11 Regression] Miscompilation with -m32 -O1 -march=i686

[cvs-commit at gcc dot gnu.org]

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

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

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

commit r9-8943-gaa42c1ac24a0427a27aec19c483780662cd150b8
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sun Sep 27 23:18:26 2020 +0200

    optabs: Don't reuse target for multi-word expansions if it overlaps
operand(s) [PR97073]

    The following testcase is miscompiled on i686-linux, because
    we try to expand a double-word bitwise logic operation with op0
    being a (mem:DI u) and target (mem:DI u+4), i.e. partial overlap, and
    thus end up with:
            movl    4(%esp), %eax
            andl    u, %eax
            movl    %eax, u+4
    ! movl u+4, %eax optimized out
            andl    8(%esp), %eax
            movl    %eax, u+8
    rather than with the desired:
            movl    4(%esp), %edx
            movl    8(%esp), %eax
            andl    u, %edx
            andl    u+4, %eax
            movl    %eax, u+8
            movl    %edx, u+4
    because the store of the first word to target overwrites the second word of
    the operand.
    expand_binop for this (and several similar places) already check for target
    == op0 or target == op1, this patch just adds reg_overlap_mentioned_p calls
    next to it.
    Pedantically, at least for some of these it might be sufficient to force
    a different target if there is overlap but target is not rtx_equal_p to
    the operand (e.g. in this bitwise logical case, but e.g. not in the shift
    cases where there is reordering), though that would go against the
    preexisting target == op? checks and the rationale that REG_EQUAL notes in
    that case isn't correct.

    2020-09-27  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/97073
            * optabs.c (expand_binop, expand_absneg_bit, expand_unop,
            expand_copysign_bit): Check reg_overlap_mentioned_p between target
            and operand(s) and if it returns true, force a pseudo as target.

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

    (cherry picked from commit a4b31d5807f2bc67c8999b3d53369cf2a5c6e1ec)

[Bug middle-end/97073] [8/9/10/11 Regression] Miscompilation with -m32 -O1 -march=i686

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:0ed1ce77f876cb05cd3e403c6c28e86fe0692f93

commit r8-10540-g0ed1ce77f876cb05cd3e403c6c28e86fe0692f93
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sun Sep 27 23:18:26 2020 +0200

    optabs: Don't reuse target for multi-word expansions if it overlaps
operand(s) [PR97073]

    The following testcase is miscompiled on i686-linux, because
    we try to expand a double-word bitwise logic operation with op0
    being a (mem:DI u) and target (mem:DI u+4), i.e. partial overlap, and
    thus end up with:
            movl    4(%esp), %eax
            andl    u, %eax
            movl    %eax, u+4
    ! movl u+4, %eax optimized out
            andl    8(%esp), %eax
            movl    %eax, u+8
    rather than with the desired:
            movl    4(%esp), %edx
            movl    8(%esp), %eax
            andl    u, %edx
            andl    u+4, %eax
            movl    %eax, u+8
            movl    %edx, u+4
    because the store of the first word to target overwrites the second word of
    the operand.
    expand_binop for this (and several similar places) already check for target
    == op0 or target == op1, this patch just adds reg_overlap_mentioned_p calls
    next to it.
    Pedantically, at least for some of these it might be sufficient to force
    a different target if there is overlap but target is not rtx_equal_p to
    the operand (e.g. in this bitwise logical case, but e.g. not in the shift
    cases where there is reordering), though that would go against the
    preexisting target == op? checks and the rationale that REG_EQUAL notes in
    that case isn't correct.

    2020-09-27  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/97073
            * optabs.c (expand_binop, expand_absneg_bit, expand_unop,
            expand_copysign_bit): Check reg_overlap_mentioned_p between target
            and operand(s) and if it returns true, force a pseudo as target.

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

    (cherry picked from commit a4b31d5807f2bc67c8999b3d53369cf2a5c6e1ec)

[Bug middle-end/97073] [8/9/10/11 Regression] Miscompilation with -m32 -O1 -march=i686

[jakub at gcc dot gnu.org]

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

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

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

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