From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9431 invoked by alias); 18 Feb 2013 12:05:58 -0000 Received: (qmail 9419 invoked by uid 22791); 18 Feb 2013 12:05:57 -0000 X-SWARE-Spam-Status: No, hits=-5.0 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,TW_TM X-Spam-Check-By: sourceware.org Received: from mail-wi0-f171.google.com (HELO mail-wi0-f171.google.com) (209.85.212.171) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 18 Feb 2013 12:05:40 +0000 Received: by mail-wi0-f171.google.com with SMTP id hn17so3367072wib.10 for ; Mon, 18 Feb 2013 04:05:39 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.181.13.175 with SMTP id ez15mr19632803wid.8.1361189138993; Mon, 18 Feb 2013 04:05:38 -0800 (PST) Received: by 10.194.56.100 with HTTP; Mon, 18 Feb 2013 04:05:38 -0800 (PST) In-Reply-To: <1360950831.3498.59.camel@gnopaine> References: <1360950831.3498.59.camel@gnopaine> Date: Mon, 18 Feb 2013 12:05:00 -0000 Message-ID: Subject: Re: [PATCH] Fix PR56321 From: Richard Biener To: Bill Schmidt Cc: gcc-patches@gcc.gnu.org, bergner@vnet.ibm.com Content-Type: text/plain; charset=ISO-8859-1 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2013-02/txt/msg00846.txt.bz2 On Fri, Feb 15, 2013 at 6:53 PM, Bill Schmidt wrote: > When we remove __builtin_pow statements as part of reassociation, we > have to unlink the associated VDEF. We've always done this when we > directly remove the statement. However, in reassociation the statements > are sometimes modified in place instead of removed, potentially leaving > one or more dangling VUSEs. This patch solves the problem by unlinking > the VDEF when the statement's operands are added to the ops list. > > Bootstrapped and regression tested on powerpc64-unknown-linux-gnu with > no new regressions. The new test case is the code that exposed the > problem in PR56321. Ok for trunk? No, that's way to complicated. The issue is that static void propagate_op_to_single_use (tree op, gimple stmt, tree *def) { ... gsi = gsi_for_stmt (stmt); gsi_remove (&gsi, true); release_defs (stmt); if (is_gimple_call (stmt)) unlink_stmt_vdef (stmt); tries to unlink the stmts VDEF after releasing it. That doesn't work. A proper fix is Index: tree-ssa-reassoc.c =================================================================== --- tree-ssa-reassoc.c (revision 196115) +++ tree-ssa-reassoc.c (working copy) @@ -1062,11 +1062,9 @@ propagate_op_to_single_use (tree op, gim if (TREE_CODE (op) != SSA_NAME) update_stmt (use_stmt); gsi = gsi_for_stmt (stmt); + unlink_stmt_vdef (stmt); gsi_remove (&gsi, true); release_defs (stmt); - - if (is_gimple_call (stmt)) - unlink_stmt_vdef (stmt); } /* Walks the linear chain with result *DEF searching for an operation I'll take care of it in a second. Thanks, Richard. > Thanks, > Bill > > > Index: gcc/testsuite/gcc.dg/tree-ssa/pr56321.c > =================================================================== > --- gcc/testsuite/gcc.dg/tree-ssa/pr56321.c (revision 0) > +++ gcc/testsuite/gcc.dg/tree-ssa/pr56321.c (revision 0) > @@ -0,0 +1,12 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O3 -ffast-math -fdump-tree-optimized" } */ > + > +float foo(int n) > +{ > + return ((2.0*n*n)/3.0+2.0*n); > +} > + > +/* { dg-final { scan-tree-dump-times "__builtin_pow" 0 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times " \\* " 2 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times " \\+ " 1 "optimized" } } */ > +/* { dg-final { cleanup-tree-dump "optimized" } } */ > Index: gcc/tree-ssa-reassoc.c > =================================================================== > --- gcc/tree-ssa-reassoc.c (revision 196053) > +++ gcc/tree-ssa-reassoc.c (working copy) > @@ -3386,6 +3386,10 @@ linearize_expr_tree (vec *ops, gi > { > add_repeat_to_ops_vec (ops, base, exponent); > gimple_set_visited (binrhsdef, true); > + // We may not physically remove the call later because > + // stmts are preferably modified in place. But we have > + // to remove any VDEF associated with the call regardless. > + unlink_stmt_vdef (binrhsdef); > } > else > add_to_ops_vec (ops, binrhs); > @@ -3396,6 +3400,10 @@ linearize_expr_tree (vec *ops, gi > { > add_repeat_to_ops_vec (ops, base, exponent); > gimple_set_visited (binlhsdef, true); > + // We may not physically remove the call later because > + // stmts are preferably modified in place. But we have > + // to remove any VDEF associated with the call regardless. > + unlink_stmt_vdef (binlhsdef); > } > else > add_to_ops_vec (ops, binlhs); > @@ -3445,6 +3453,10 @@ linearize_expr_tree (vec *ops, gi > { > add_repeat_to_ops_vec (ops, base, exponent); > gimple_set_visited (SSA_NAME_DEF_STMT (binrhs), true); > + // We may not physically remove the call later because > + // stmts are preferably modified in place. But we have > + // to remove any VDEF associated with the call regardless. > + unlink_stmt_vdef (SSA_NAME_DEF_STMT (binrhs)); > } > else > add_to_ops_vec (ops, binrhs); > >