[Bug tree-optimization/58805] [4.8/4.9 Regression] Inline assembly wrongly optimized out when inside a conditional

["rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

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).