public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional
@ 2013-10-19 11:01 Jeremie.Detrey at loria dot fr
  2013-10-19 11:52 ` [Bug inline-asm/58805] " mikpelinux at gmail dot com
                   ` (16 more replies)
  0 siblings, 17 replies; 18+ messages in thread
From: Jeremie.Detrey at loria dot fr @ 2013-10-19 11:01 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

            Bug ID: 58805
           Summary: Inline assembly wrongly optimized out when inside a
                    conditional
           Product: gcc
           Version: 4.8.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: inline-asm
          Assignee: unassigned at gcc dot gnu.org
          Reporter: Jeremie.Detrey at loria dot fr

Created attachment 31050
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31050&action=edit
Minimal example to reproduce the bug.

When compiling the attached example with GCC 4.8.1 (on an x86_64 ArchLinux)
using -O2 optimization, the `else' branch of the conditional in foo() is
optimized out: GCC only generates code for the first branch, and does not test
whether the value of `n' is zero or not. This can be seen in the following
assembly output:

$ gcc -O2 -S bug.c -o-
        .file   "bug.c"
        .text
        .p2align 4,,15
        .globl  foo
        .type   foo, @function
foo:
.LFB1:
        .cfi_startproc
#APP
# 4 "bug.c" 1
        movq $42, %rdx
        movq %rdx, %rax

# 0 "" 2
#NO_APP
        movq    %rax, (%rsi)
        ret
        .cfi_endproc
.LFE1:
        .size   foo, .-foo
        .ident  "GCC: (GNU) 4.8.1"
        .section        .note.GNU-stack,"",@progbits

This bug does not appear when compiling with GCC 4.7.3 (on an x86_64 Ubuntu
13.04, also with -O2 optimization): the two branches are generated, and the
test on `n' is performed to select which branch to jump to:

$ gcc -O2 -S bug.c -o-
        .file   "bug.c"
        .text
        .p2align 4,,15
        .globl  foo
        .type   foo, @function
foo:
.LFB1:
        .cfi_startproc
        testl   %edi, %edi
        je      .L5
#APP
# 4 "bug.c" 1
        movq $42, %rcx
        movq %rcx, %rax

# 0 "" 2
#NO_APP
        movq    %rax, (%rdx)
        ret
        .p2align 4,,10
        .p2align 3
.L5:
#APP
# 4 "bug.c" 1
        movq $42, %rdx
        movq %rdx, %rax

# 0 "" 2
#NO_APP
        movq    %rax, (%rsi)
        ret
        .cfi_endproc
.LFE1:
        .size   foo, .-foo
        .ident  "GCC: (Ubuntu/Linaro 4.7.3-1ubuntu1) 4.7.3"
        .section        .note.GNU-stack,"",@progbits

This bug disappears (in GCC 4.8.1) when either declaring the inline asm block
as `volatile', or removing the intermediate variable `t' and assigning 42
directly to `%[r]'.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug inline-asm/58805] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
@ 2013-10-19 11:52 ` mikpelinux at gmail dot com
  2013-10-19 18:02 ` [Bug inline-asm/58805] [4.8/4.9 Regression] " pinskia at gcc dot gnu.org
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: mikpelinux at gmail dot com @ 2013-10-19 11:52 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

--- Comment #1 from Mikael Pettersson <mikpelinux at gmail dot com> ---
Let me quote a section of the manual:

"If an `asm' has output operands, GCC assumes for optimization purposes
the instruction has no side effects except to change the output
operands.  This does not mean instructions with a side effect cannot be
used, but you must be careful, because the compiler may eliminate them
if the output operands aren't used, or move them out of loops, or
replace two with one if they constitute a common subexpression.  Also,
if your instruction does have a side effect on a variable that otherwise
appears not to change, the old value of the variable may be reused later
if it happens to be found in a register.

 You can prevent an `asm' instruction from being deleted by writing the
keyword `volatile' after the `asm'."

In your test case the asm has output operands feeding into write-only local
variables, but no memory clobbers and no volatile annotation.  Therefore GCC is
allowed to remove the asm completely.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug inline-asm/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
  2013-10-19 11:52 ` [Bug inline-asm/58805] " mikpelinux at gmail dot com
@ 2013-10-19 18:02 ` pinskia at gcc dot gnu.org
  2013-10-19 19:15 ` mikpelinux at gmail dot com
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2013-10-19 18:02 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-10-19
   Target Milestone|---                         |4.8.3
            Summary|Inline assembly wrongly     |[4.8/4.9 Regression] Inline
                   |optimized out when inside a |assembly wrongly optimized
                   |conditional                 |out when inside a
                   |                            |conditional
     Ever confirmed|0                           |1

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed, this is a bug as the write to *x/*y should depend on the value of n.

> In your test case the asm has output operands feeding into write-only local
> variables, but no memory clobbers and no volatile annotation.

There is a memory write with `[r] "=r" (*r)'.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug inline-asm/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
  2013-10-19 11:52 ` [Bug inline-asm/58805] " mikpelinux at gmail dot com
  2013-10-19 18:02 ` [Bug inline-asm/58805] [4.8/4.9 Regression] " pinskia at gcc dot gnu.org
@ 2013-10-19 19:15 ` mikpelinux at gmail dot com
  2013-10-19 22:08 ` segher at gcc dot gnu.org
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: mikpelinux at gmail dot com @ 2013-10-19 19:15 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

--- Comment #3 from Mikael Pettersson <mikpelinux at gmail dot com> ---
(In reply to Andrew Pinski from comment #2)
> > In your test case the asm has output operands feeding into write-only local
> > variables, but no memory clobbers and no volatile annotation.
> 
> There is a memory write with `[r] "=r" (*r)'.

Oops.  My bad, got confused by all the 'r's that denote different things :-(


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug inline-asm/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (2 preceding siblings ...)
  2013-10-19 19:15 ` mikpelinux at gmail dot com
@ 2013-10-19 22:08 ` segher at gcc dot gnu.org
  2013-10-19 22:39 ` pinskia at gcc dot gnu.org
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: segher at gcc dot gnu.org @ 2013-10-19 22:08 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

Segher Boessenkool <segher at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |UNCONFIRMED
                 CC|                            |segher at gcc dot gnu.org
     Ever confirmed|1                           |0

--- Comment #4 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #2)
> There is a memory write with `[r] "=r" (*r)'.

That's not a memory write (=r vs. =m).


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug inline-asm/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (3 preceding siblings ...)
  2013-10-19 22:08 ` segher at gcc dot gnu.org
@ 2013-10-19 22:39 ` pinskia at gcc dot gnu.org
  2013-10-19 22:40 ` pinskia at gcc dot gnu.org
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2013-10-19 22:39 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #4)
> (In reply to Andrew Pinski from comment #2)
> > There is a memory write with `[r] "=r" (*r)'.
> 
> That's not a memory write (=r vs. =m).

The inline-asm itself might not be a memory write but the memory write happens
after.  That is:
asm("":"r"(*a));
is the same as:
asm("":"r"(temp));
*a = temp;


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug inline-asm/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (4 preceding siblings ...)
  2013-10-19 22:39 ` pinskia at gcc dot gnu.org
@ 2013-10-19 22:40 ` pinskia at gcc dot gnu.org
  2013-10-19 22:59 ` segher at gcc dot gnu.org
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: pinskia at gcc dot gnu.org @ 2013-10-19 22:40 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I mean:
asm("":"=r"(*a));
is the same as:
asm("":"=r"(temp));
*a = temp;

(forgot the = in the constraint in the first place).


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug inline-asm/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (5 preceding siblings ...)
  2013-10-19 22:40 ` pinskia at gcc dot gnu.org
@ 2013-10-19 22:59 ` segher at gcc dot gnu.org
  2013-10-19 23:07 ` segher at gcc dot gnu.org
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: segher at gcc dot gnu.org @ 2013-10-19 22:59 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

--- Comment #7 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #6)
> I mean:
> asm("":"=r"(*a));
> is the same as:
> asm("":"=r"(temp));
> *a = temp;

I'm not convinced it is the same.  In the first case,
the compiler doesn't seem to be aware there is a
memory write; should it?  The asm doesn't say there
is one: it says you write to a register.  So the two
arms look the same and are merged, the memory write
is later done by reload.

But the compiler should at least warn.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug inline-asm/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (6 preceding siblings ...)
  2013-10-19 22:59 ` segher at gcc dot gnu.org
@ 2013-10-19 23:07 ` segher at gcc dot gnu.org
  2013-10-21  9:11 ` [Bug tree-optimization/58805] " vries at gcc dot gnu.org
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: segher at gcc dot gnu.org @ 2013-10-19 23:07 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

--- Comment #8 from Segher Boessenkool <segher at gcc dot gnu.org> ---
It's the ssa-pre pass that merges the arms.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (7 preceding siblings ...)
  2013-10-19 23:07 ` segher at gcc dot gnu.org
@ 2013-10-21  9:11 ` vries at gcc dot gnu.org
  2013-10-21 10:14 ` vries at gcc dot gnu.org
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2013-10-21  9:11 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

--- Comment #10 from vries at gcc dot gnu.org ---
I've tried out the example from comment 9.

Tree-tail-merge considers the statements without effect because:
...
(gdb) call debug_gimple_stmt (stmt)
# .MEM_10 = VDEF <.MEM_3(D)>
__asm__("movq $42, %0
    movq %0, %1
    " : "t" "=&r" t_8, "r" "=r" *x_4(D));
(gdb) call gimple_has_side_effects (stmt)
$2 = false
...
and tree-ssa-tail-merge.c:stmt_local_def will return true.

The side effect of the asm is in the expression for the output operand, so 
gimple_has_side_effects should return true.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (8 preceding siblings ...)
  2013-10-21  9:11 ` [Bug tree-optimization/58805] " vries at gcc dot gnu.org
@ 2013-10-21 10:14 ` vries at gcc dot gnu.org
  2013-10-21 10:16 ` rguenth at gcc dot gnu.org
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2013-10-21 10:14 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

vries at gcc dot gnu.org changed:

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

--- Comment #11 from vries at gcc dot gnu.org ---
Using this tentative patch, tree-tail-merge leaves the test-case alone:
...
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 3ddceb9..7e00037 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -2394,6 +2394,13 @@ gimple_copy (gimple stmt)
   return copy;
 }

+static bool
+return_true_bool_gimple_tree_voidptr (gimple s ATTRIBUTE_UNUSED,
+                                     tree t ATTRIBUTE_UNUSED,
+                                     void *p ATTRIBUTE_UNUSED)
+{
+  return true;
+}

 /* Return true if statement S has side-effects.  We consider a
    statement to have side effects if:
@@ -2413,9 +2420,16 @@ gimple_has_side_effects (const_gimple s)
   if (gimple_has_volatile_ops (s))
     return true;

-  if (gimple_code (s) == GIMPLE_ASM
-      && gimple_asm_volatile_p (s))
-    return true;
+  if (gimple_code (s) == GIMPLE_ASM)
+    {
+      if (gimple_asm_volatile_p (s))
+       return true;
+
+      if (walk_stmt_load_store_addr_ops ((gimple)s, NULL, NULL,
+                                        return_true_bool_gimple_tree_voidptr,
+                                        NULL))
+      return true;
+    }

   if (is_gimple_call (s))
     {
...


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (9 preceding siblings ...)
  2013-10-21 10:14 ` vries at gcc dot gnu.org
@ 2013-10-21 10:16 ` rguenth at gcc dot gnu.org
  2013-10-21 12:24 ` vries at gcc dot gnu.org
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: rguenth at gcc dot gnu.org @ 2013-10-21 10:16 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to vries from comment #10)
> I've tried out the example from comment 9.
> 
> Tree-tail-merge considers the statements without effect because:
> ...
> (gdb) call debug_gimple_stmt (stmt)
> # .MEM_10 = VDEF <.MEM_3(D)>
> __asm__("movq $42, %0
> 	movq %0, %1
> 	" : "t" "=&r" t_8, "r" "=r" *x_4(D));
> (gdb) call gimple_has_side_effects (stmt)
> $2 = false
> ...
> and tree-ssa-tail-merge.c:stmt_local_def will return true.
> 
> The side effect of the asm is in the expression for the output operand, so 
> gimple_has_side_effects should return true.

No it shouldn't.  It should return true if the stmt has side-effects that
are _not_ explicit in the statement.  This side-effect is explicitely there.
>From gcc-bugs-return-432341-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Mon Oct 21 10:18:28 2013
Return-Path: <gcc-bugs-return-432341-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 12297 invoked by alias); 21 Oct 2013 10:18:28 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 12243 invoked by uid 48); 21 Oct 2013 10:18:25 -0000
From: "rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug tree-optimization/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
Date: Mon, 21 Oct 2013 10:18:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: tree-optimization
X-Bugzilla-Version: 4.8.1
X-Bugzilla-Keywords: wrong-code
X-Bugzilla-Severity: normal
X-Bugzilla-Who: rguenth at gcc dot gnu.org
X-Bugzilla-Status: NEW
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 4.8.3
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-58805-4-nrW6augChN@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-58805-4@http.gcc.gnu.org/bugzilla/>
References: <bug-58805-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-10/txt/msg01485.txt.bz2
Content-length: 1168

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

--- Comment #13 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #12)
> (In reply to vries from comment #10)
> > I've tried out the example from comment 9.
> > 
> > Tree-tail-merge considers the statements without effect because:
> > ...
> > (gdb) call debug_gimple_stmt (stmt)
> > # .MEM_10 = VDEF <.MEM_3(D)>
> > __asm__("movq $42, %0
> > 	movq %0, %1
> > 	" : "t" "=&r" t_8, "r" "=r" *x_4(D));
> > (gdb) call gimple_has_side_effects (stmt)
> > $2 = false
> > ...
> > and tree-ssa-tail-merge.c:stmt_local_def will return true.
> > 
> > The side effect of the asm is in the expression for the output operand, so 
> > gimple_has_side_effects should return true.
> 
> No it shouldn't.  It should return true if the stmt has side-effects that
> are _not_ explicit in the statement.  This side-effect is explicitely there.

That said, it doesn't return true for the call foo(x) in

struct X { int a[256]; };
struct X __attribute__((const)) foo ();

void bar (void)
{
  struct X x;
  x = foo ();
}

either, even though the call has a (explicit) store.
>From gcc-bugs-return-432342-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Mon Oct 21 10:38:42 2013
Return-Path: <gcc-bugs-return-432342-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 23851 invoked by alias); 21 Oct 2013 10:38:41 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 23796 invoked by uid 48); 21 Oct 2013 10:38:36 -0000
From: "rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug tree-optimization/58794] [4.8/4.9 Regression] ICE in set_lattice_value, at tree-ssa-ccp.c:455 on x86_64-linux-gnu (at -O1, -O2, and -O3)
Date: Mon, 21 Oct 2013 10:38:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: tree-optimization
X-Bugzilla-Version: 4.9.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: rguenth at gcc dot gnu.org
X-Bugzilla-Status: ASSIGNED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: rguenth at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 4.8.3
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: cc
Message-ID: <bug-58794-4-QnvR8MWskK@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-58794-4@http.gcc.gnu.org/bugzilla/>
References: <bug-58794-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-10/txt/msg01486.txt.bz2
Content-length: 3358

http://gcc.gnu.org/bugzilla/show_bug.cgi?idX794

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

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #3)
> The issue being that &a.f and &a.f are not equal because even with
> OEP_CONSTANT_ADDRESS_OF set we get into
>
>         case COMPONENT_REF:
>           /* Handle operand 2 the same as for ARRAY_REF.  Operand 0
>              may be NULL when we're called to compare MEM_EXPRs.  */
>           if (!OP_SAME_WITH_NULL (0))
>             return 0;
>           flags &= ~OEP_CONSTANT_ADDRESS_OF;
>           return OP_SAME (1) && OP_SAME_WITH_NULL (2);
>
> and thus drop it before comparing the two FIELD_DECLs which have
> TREE_SIDE_EFFECTS set.  Fixed with
>
> Index: gcc/fold-const.c
> ==================================================================> --- gcc/fold-const.c    (revision 203886)
> +++ gcc/fold-const.c    (working copy)
> @@ -2715,10 +2715,11 @@ operand_equal_p (const_tree arg0, const_
>         case COMPONENT_REF:
>           /* Handle operand 2 the same as for ARRAY_REF.  Operand 0
>              may be NULL when we're called to compare MEM_EXPRs.  */
> -         if (!OP_SAME_WITH_NULL (0))
> +         if (!OP_SAME_WITH_NULL (0)
> +             || !OP_SAME (1))
>             return 0;
>           flags &= ~OEP_CONSTANT_ADDRESS_OF;
> -         return OP_SAME (1) && OP_SAME_WITH_NULL (2);
> +         return OP_SAME_WITH_NULL (2);
>
>         case BIT_FIELD_REF:
>           if (!OP_SAME (0))
>
> I spotted this earlier but for some reason chickeded out to make this
> change.  Hmm.  Trying to remember why ...

I think it was because the offsets in the FIELD_DECL could have side-effects.
So you could argue that it is the C/C++ frontends that shouldn't set
TREE_SIDE_EFFECTS on the FIELD_DECL if that isn't the case for tem.
But of course then this bit should vanish again at the point we lower
COMPONENT_REFs to populate their operand 2 with any non-constant offfsets
(and thus side-effects).

For C and C++ TREE_SIDE_EFFECTS comes from

c_apply_type_quals_to_decl (type_quals=2, decl=<field_decl 0x7ffff6e3b130 f2>)
    at /space/rguenther/src/svn/trunk/gcc/c-family/c-common.c:4719
4719          TREE_THIS_VOLATILE (decl) = 1;

and ultimately grokdeclarator.

I don't see a way out of this as we possibly overload TREE_SIDE_EFFECTS with
both, volatility of the field itself (ok to skip with OEP_CONSTANT_ADDRESS_OF)
and side-effects inside DECL_FIELD_OFFSET (a volatile load therein, or
other side-effects).

I'm sure that we can build such FIELD_DECL only with Ada though, so, Eric,
can you provide a testcase where that happens - thus, that shows that
side-effects cannot be ignored here by for example comparing
&f.x and &f.x for a case where that is not equal?  I think we need to
concern ourselves only with mutating side-effects, not a volatile load.

The question is whether the patch is ok as-is or if I have to make
behavior dependent on is_gimple_form (ugh).  A testcase that breaks
if not guarding it that way would be nice to have (I'll check if anything
existing triggers).


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (10 preceding siblings ...)
  2013-10-21 10:16 ` rguenth at gcc dot gnu.org
@ 2013-10-21 12:24 ` vries at gcc dot gnu.org
  2013-10-21 12:41 ` rguenth at gcc dot gnu.org
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2013-10-21 12:24 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

--- Comment #14 from vries at gcc dot gnu.org ---
> No it shouldn't.  It should return true if the stmt has side-effects that
> are _not_ explicit in the statement.  This side-effect is explicitely there.

Hmm, a rather deceptive name then, if gimple_stmt_has_side_effects does not in
fact tell whether a gimple statement has side-effects or not.
I wonder if any other use sites are mistaken in the semantics, and what could
be a better name. 

A better name according to your description could be
gimple_stmt_has_implicit_side_effects, but I'm not sure I understand how to
differentiate between implicit and explicit here. Marking an asm with volatile,
is that implicit or explicit, and why?

AFAIU, the name gimple_stmt_has_call_or_volatile_side_effects reflects what the
implementation does.

Anyways, this tentative patch fixes the problem in tail-merge:
...
Author: Tom de Vries <tom@codesourcery.com>
Date:   Mon Oct 21 13:40:28 2013 +0200

        * tree-ssa-tail-merge.c (stmt_local_def): Improve side-effect check.

diff --git a/gcc/testsuite/gcc.dg/pr58805.c b/gcc/testsuite/gcc.dg/pr58805.c
new file mode 100644
index 0000000..6e6eba5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr58805.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -ftree-tail-merge -fdump-tree-pre" } */
+
+static inline void bar(unsigned long *r)
+{
+  unsigned long t;
+  __asm__ (
+    "movq $42, %[t]\n\t"
+    "movq %[t], %[r]\n\t"
+    : [t] "=&r" (t), [r] "=r" (*r)
+  );
+}
+
+void foo(int n, unsigned long *x, unsigned long *y)
+{
+  if (n == 0)
+    bar(x);
+  else
+    bar(y);
+}
+
+/* { dg-final { scan-tree-dump-times "__asm__" 2 "pre"} } */
+/* { dg-final { cleanup-tree-dump "pre" } } */
diff --git a/gcc/tree-ssa-tail-merge.c b/gcc/tree-ssa-tail-merge.c
index 9094935..0090da6 100644
--- a/gcc/tree-ssa-tail-merge.c
+++ b/gcc/tree-ssa-tail-merge.c
@@ -300,7 +300,8 @@ stmt_local_def (gimple stmt)
   tree val;
   def_operand_p def_p;

-  if (gimple_has_side_effects (stmt))
+  if (gimple_has_side_effects (stmt)
+      || gimple_vdef (stmt) != NULL_TREE)
     return false;

   def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
...

I'll test this one.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (11 preceding siblings ...)
  2013-10-21 12:24 ` vries at gcc dot gnu.org
@ 2013-10-21 12:41 ` rguenth at gcc dot gnu.org
  2013-10-21 14:43 ` vries at gcc dot gnu.org
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: rguenth at gcc dot gnu.org @ 2013-10-21 12:41 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

--- Comment #15 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to vries from comment #14)
> > No it shouldn't.  It should return true if the stmt has side-effects that
> > are _not_ explicit in the statement.  This side-effect is explicitely there.
> 
> Hmm, a rather deceptive name then, if gimple_stmt_has_side_effects does not
> in fact tell whether a gimple statement has side-effects or not.
> I wonder if any other use sites are mistaken in the semantics, and what
> could be a better name. 

A side-effect is by definition an effect that happens "on the side", so
I think the name quite matches.

> A better name according to your description could be
> gimple_stmt_has_implicit_side_effects, but I'm not sure I understand how to
> differentiate between implicit and explicit here. Marking an asm with
> volatile, is that implicit or explicit, and why?

Well, is in a = b + c the computation of a + c a side-effect or not?
Is in a = b the assignment to 'a' a side-effect or not.  Neither I would
argue.

> AFAIU, the name gimple_stmt_has_call_or_volatile_side_effects reflects what
> the implementation does.

No, for calls the side-effects of the callee body are not explicit (unless
it is pure or const).

> Anyways, this tentative patch fixes the problem in tail-merge:
> ...
> Author: Tom de Vries <tom@codesourcery.com>
> Date:   Mon Oct 21 13:40:28 2013 +0200
> 
>         * tree-ssa-tail-merge.c (stmt_local_def): Improve side-effect check.
> 
> diff --git a/gcc/testsuite/gcc.dg/pr58805.c b/gcc/testsuite/gcc.dg/pr58805.c
> new file mode 100644
> index 0000000..6e6eba5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr58805.c
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -ftree-tail-merge -fdump-tree-pre" } */
> +
> +static inline void bar(unsigned long *r)
> +{
> +  unsigned long t;
> +  __asm__ (
> +    "movq $42, %[t]\n\t"
> +    "movq %[t], %[r]\n\t"
> +    : [t] "=&r" (t), [r] "=r" (*r)
> +  );
> +}
> +
> +void foo(int n, unsigned long *x, unsigned long *y)
> +{
> +  if (n == 0)
> +    bar(x);
> +  else
> +    bar(y);
> +}
> +
> +/* { dg-final { scan-tree-dump-times "__asm__" 2 "pre"} } */
> +/* { dg-final { cleanup-tree-dump "pre" } } */
> diff --git a/gcc/tree-ssa-tail-merge.c b/gcc/tree-ssa-tail-merge.c
> index 9094935..0090da6 100644
> --- a/gcc/tree-ssa-tail-merge.c
> +++ b/gcc/tree-ssa-tail-merge.c
> @@ -300,7 +300,8 @@ stmt_local_def (gimple stmt)
>    tree val;
>    def_operand_p def_p;
>  
> -  if (gimple_has_side_effects (stmt))
> +  if (gimple_has_side_effects (stmt)
> +      || gimple_vdef (stmt) != NULL_TREE)
>      return false;
>
>    def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
> ...
> 
> I'll test this one.

Looks good to me.  Btw, you'd have had the same issue with the aggregate
return of a pure/const function call, no?


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (12 preceding siblings ...)
  2013-10-21 12:41 ` rguenth at gcc dot gnu.org
@ 2013-10-21 14:43 ` vries at gcc dot gnu.org
  2013-10-23 13:26 ` vries at gcc dot gnu.org
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2013-10-21 14:43 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

--- Comment #16 from vries at gcc dot gnu.org ---
> Btw, you'd have had the same issue with the aggregate
> return of a pure/const function call, no?

For this bug to trigger, we need a gimple statement:
- without side-effects
- with one SSA_OP_DEF result (which also needs to be an SSA_NAME)
- with an SSA_OP_VDEF result

The case you mention doesn't have an SSA_OP_DEF, so that one is caught by this
check:
...
  def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
  if (def_p == NULL)
    return false;
...


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (13 preceding siblings ...)
  2013-10-21 14:43 ` vries at gcc dot gnu.org
@ 2013-10-23 13:26 ` vries at gcc dot gnu.org
  2013-10-23 19:17 ` vries at gcc dot gnu.org
  2013-10-24  8:47 ` vries at gcc dot gnu.org
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2013-10-23 13:26 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

--- Comment #17 from vries at gcc dot gnu.org ---
Author: vries
Date: Wed Oct 23 13:26:45 2013
New Revision: 203973

URL: http://gcc.gnu.org/viewcvs?rev=203973&root=gcc&view=rev
Log:
Add missing check in stmt_local_def for tail-merge.

2013-10-22  Tom de Vries  <tom@codesourcery.com>

    PR tree-optimization/58805
    * tree-ssa-tail-merge.c (stmt_local_def): Add gimple_vdef check.

    * gcc.dg/pr58805.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/pr58805.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-tail-merge.c


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (14 preceding siblings ...)
  2013-10-23 13:26 ` vries at gcc dot gnu.org
@ 2013-10-23 19:17 ` vries at gcc dot gnu.org
  2013-10-24  8:47 ` vries at gcc dot gnu.org
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2013-10-23 19:17 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

--- Comment #18 from vries at gcc dot gnu.org ---
Author: vries
Date: Wed Oct 23 19:16:55 2013
New Revision: 203990

URL: http://gcc.gnu.org/viewcvs?rev=203990&root=gcc&view=rev
Log:
Add missing check in stmt_local_def for tail-merge.

2013-10-23  Tom de Vries  <tom@codesourcery.com>

    PR tree-optimization/58805
    * tree-ssa-tail-merge.c (stmt_local_def): Add gimple_vdef check.

    * gcc.dg/pr58805.c: New test.

Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.dg/pr58805.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_8-branch/gcc/tree-ssa-tail-merge.c


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Bug tree-optimization/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional
  2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
                   ` (15 preceding siblings ...)
  2013-10-23 19:17 ` vries at gcc dot gnu.org
@ 2013-10-24  8:47 ` vries at gcc dot gnu.org
  16 siblings, 0 replies; 18+ messages in thread
From: vries at gcc dot gnu.org @ 2013-10-24  8:47 UTC (permalink / raw)
  To: gcc-bugs

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58805

vries at gcc dot gnu.org changed:

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

--- Comment #19 from vries at gcc dot gnu.org ---
Patch committed to 4.8 and trunk, test-case included. Marking resolved-fixed.


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2013-10-24  8:47 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-19 11:01 [Bug inline-asm/58805] New: Inline assembly wrongly optimized out when inside a conditional Jeremie.Detrey at loria dot fr
2013-10-19 11:52 ` [Bug inline-asm/58805] " mikpelinux at gmail dot com
2013-10-19 18:02 ` [Bug inline-asm/58805] [4.8/4.9 Regression] " pinskia at gcc dot gnu.org
2013-10-19 19:15 ` mikpelinux at gmail dot com
2013-10-19 22:08 ` segher at gcc dot gnu.org
2013-10-19 22:39 ` pinskia at gcc dot gnu.org
2013-10-19 22:40 ` pinskia at gcc dot gnu.org
2013-10-19 22:59 ` segher at gcc dot gnu.org
2013-10-19 23:07 ` segher at gcc dot gnu.org
2013-10-21  9:11 ` [Bug tree-optimization/58805] " vries at gcc dot gnu.org
2013-10-21 10:14 ` vries at gcc dot gnu.org
2013-10-21 10:16 ` rguenth at gcc dot gnu.org
2013-10-21 12:24 ` vries at gcc dot gnu.org
2013-10-21 12:41 ` rguenth at gcc dot gnu.org
2013-10-21 14:43 ` vries at gcc dot gnu.org
2013-10-23 13:26 ` vries at gcc dot gnu.org
2013-10-23 19:17 ` vries at gcc dot gnu.org
2013-10-24  8:47 ` vries at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).