[Bug c/65217] New: __builtin_unreachable in if statement causes bad assembly generation

[Bug c/65217] New: __builtin_unreachable in if statement causes bad assembly generation

[sumnon at cj3 dot org]

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

            Bug ID: 65217
           Summary: __builtin_unreachable in if statement causes bad
                    assembly generation
           Product: gcc
           Version: 4.9.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sumnon at cj3 dot org

Test case:

int expected(int n) {
    return n;
}

int test(int n) {
    if ((n & -n) != n) {
        __builtin_unreachable();
    }
    return n;
}

gcc -O3 -S test.c generates this:

    .file    "test.c"
    .text
    .p2align 4,,15
    .globl    expected
    .type    expected, @function
expected:
.LFB0:
    .cfi_startproc
    movl    %edi, %eax
    ret
    .cfi_endproc
.LFE0:
    .size    expected, .-expected
    .p2align 4,,15
    .globl    test
    .type    test, @function
test:
.LFB1:
    .cfi_startproc
    movl    %edi, %eax
    negl    %eax
    andl    %edi, %eax
    ret
    .cfi_endproc
.LFE1:
    .size    test, .-test
    .ident    "GCC: (GNU) 4.8.3 20140911 (Red Hat 4.8.3-7)"
    .section    .note.GNU-stack,"",@progbits

These functions should generate the same assembly, since the if statement in
function test does nothing that affects the return value.

I have and tested this with gcc 4.8.3; a friend tested this with gcc 4.9.2 and
got the same assembly.

[Bug c/65217] __builtin_unreachable in if statement causes bad assembly generation

[pinskia at gcc dot gnu.org]

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

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

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed on the trunk for GCC 5:
expected:
.LFB0:
    .cfi_startproc
    movl    %edi, %eax
    ret
...
test:
.LFB1:
    .cfi_startproc
    movl    %edi, %eax
    ret

[Bug c/65217] __builtin_unreachable in if statement causes bad assembly generation

[hjl at gcc dot gnu.org]

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

--- Comment #2 from hjl at gcc dot gnu.org <hjl at gcc dot gnu.org> ---
Author: hjl
Date: Thu Feb 26 14:17:06 2015
New Revision: 221010

URL: https://gcc.gnu.org/viewcvs?rev=221010&root=gcc&view=rev
Log:
Add a testcase for PR target/65217

    PR target/65217
    * gcc.target/i386/pr65217.c: New.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr65217.c
Modified:
    trunk/gcc/testsuite/ChangeLog

[Bug tree-optimization/65217] __builtin_unreachable in if statement causes bad assembly generation

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization, xfail
             Status|RESOLVED                    |REOPENED
   Last reconfirmed|                            |2015-04-27
                 CC|                            |law at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org
          Component|c                           |tree-optimization
         Resolution|FIXED                       |---
     Ever confirmed|0                           |1

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
It was only fixed by luck due to us folding all stmts in forwprop which turns

  if (_3 != n_1(D))

into the canonical

  if (n_1(D) != _3)

and then DOM recording the "proper" equality via record_equality by _always_
swapping operands due to

  else if (is_gimple_min_invariant (x)
           /* ???  When threading over backedges the following is important
              for correctness.  See PR61757.  */
           || (loop_depth_of_name (x) <= loop_depth_of_name (y)))

and its '==' part.

For non-canonical order (in GCC 4.9 and earlier) that swapping turned it into
the unfortunate direction.

That equality decides whether we propagate _3 into the return value on the
else branch (where n_1(D) == _3) or leave it as n_1(D) (good for this case).

I am about to commit a patch that does not perform this kind of random swapping
(and will XFAIL the testcase).

[Bug tree-optimization/65217] __builtin_unreachable in if statement causes bad assembly generation

[law at gcc dot gnu.org]

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

--- Comment #4 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Author: law
Date: Tue Apr 28 04:01:28 2015
New Revision: 222499

URL: https://gcc.gnu.org/viewcvs?rev=222499&root=gcc&view=rev
Log:
        PR tree-optimization/65217
        * tree-ssa-dom.c (record_equality): Given two SSA_NAMEs, if just one
        of them has a single use, make sure it is the LHS of the implied
        copy.

        PR tree-optimization/65217
        * gcc.target/i386/pr65217.c: Remove XFAIL.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.target/i386/pr65217.c
    trunk/gcc/tree-ssa-dom.c

[Bug tree-optimization/65217] __builtin_unreachable in if statement causes bad assembly generation

[law at redhat dot com]

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

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
                 CC|                            |law at redhat dot com
         Resolution|---                         |FIXED

--- Comment #5 from Jeffrey A. Law <law at redhat dot com> ---
Resolved again :-)