From patchwork Fri Nov 1 11:11:07 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 287789 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 470CE2C00CE for ; Fri, 1 Nov 2013 22:11:25 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; q=dns; s=default; b=MY+lOOTdfmi5l62heEvfRTPtmaCgr 6bIqHXqLH7beb9k00tI9Nr6We+IrSNb6mJTxcdQMFOllm1YPgO+0lAQI7zkUUUZy xOpLcGURRdexZu2pNTy20QlZh4n4hufKFXbVY+AdqNHfQ0PDFl4t6rX001c1lPii wxjrIzBIsyeoa4= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; s=default; bh=EFr+LkUVk+G1dFflM8q935SHo2o=; b=rQG yc2tpO1pzYumM0GPgTBDqw0GvQ3hLLb+ueySZdbRE6RZwD3E0pL3uNwRpmiftsps katHSX+otxrb9roLvu6nx1p5HIRQ8preC1fzXAI+HE+9jCR0vF+QOEQukGZp8MVt IEyyS3pABGQNIjsC51zHynzlboTajdd/N8jU8PSQ= Received: (qmail 8421 invoked by alias); 1 Nov 2013 11:11:15 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 8404 invoked by uid 89); 1 Nov 2013 11:11:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.3 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 01 Nov 2013 11:11:12 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rA1BBAKI023165 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 1 Nov 2013 07:11:11 -0400 Received: from tucnak.zalov.cz (vpn1-6-45.ams2.redhat.com [10.36.6.45]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id rA1BB98H014350 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 1 Nov 2013 07:11:10 -0400 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.14.7/8.14.7) with ESMTP id rA1BB8ax007346; Fri, 1 Nov 2013 12:11:08 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.14.7/8.14.7/Submit) id rA1BB7Fi007345; Fri, 1 Nov 2013 12:11:07 +0100 Date: Fri, 1 Nov 2013 12:11:07 +0100 From: Jakub Jelinek To: Richard Biener , Jeff Law Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix up reassoc (PR tree-optimization/58946) Message-ID: <20131101111107.GB27813@tucnak.zalov.cz> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Hi! My recent reassoc patch caused following regression (though, it only started failing on this testcase with Andrew's ifcombine changes). The issue is that update_ops relies on walking the same stmts as get_ops did, and uses has_single_uses (either directly or indirectly through is_reassociable_op). optimize_range_tests itself doesn't change the IL except for inserting new stmts using values on which get_ops already didn't recurse (either because they were multiple uses or non-reassociable). The problem in the testcase is when optimizing a GIMPLE_COND directly, there is no guarantee of single use, we treat the condition as the starting point of init_range_info and thus SSA_NAME != 0 or SSA_NAME == 0 etc. and that is just fine if SSA_NAME has multiple uses, so if we first change the condition to something else (as instructed by the changed ops[i]->op value from NULL to some SSA_NAME), we might turn something update_ops looks at from multiple uses into single use. This patch fixes it by doing all the update_ops calls before changing GIMPLE_CONDs themselves. I believe it is safe, update_ops will walk only single use SSA_NAMEs and thus they occur only in the single particular update_ops call, and never removes anything, only adds new stmt (which can make single use SSA_NAMEs into multiple use, but that happened after we've walked that originally single use exactly ones from the single use), and GIMPLE_COND adjustments never use has_single_use, thus they can be safely done after all update_ops have been called. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2013-11-01 Jakub Jelinek PR tree-optimization/58946 * tree-ssa-reassoc.c (maybe_optimize_range_tests): Update all bbs with bbinfo[idx].op != NULL before all blocks with bbinfo[idx].op == NULL. * gcc.c-torture/compile/pr58946.c: New test. Jakub --- gcc/tree-ssa-reassoc.c.jj 2013-10-24 10:19:21.000000000 +0200 +++ gcc/tree-ssa-reassoc.c 2013-11-01 09:23:09.264615181 +0100 @@ -2657,6 +2657,7 @@ maybe_optimize_range_tests (gimple stmt) edge e; vec ops = vNULL; vec bbinfo = vNULL; + bool any_changes = false; /* Consider only basic blocks that end with GIMPLE_COND or a cast statement satisfying final_range_test_p. All @@ -2870,41 +2871,31 @@ maybe_optimize_range_tests (gimple stmt) break; } if (ops.length () > 1) + any_changes = optimize_range_tests (ERROR_MARK, &ops); + if (any_changes) { unsigned int idx; - bool any_changes = optimize_range_tests (ERROR_MARK, &ops); - for (bb = last_bb, idx = 0; any_changes; bb = single_pred (bb), idx++) + /* update_ops relies on has_single_use predicates returning the + same values as it did during get_ops earlier. Additionally it + never removes statements, only adds new ones and it should walk + from the single imm use and check the predicate already before + making those changes. + On the other side, the handling of GIMPLE_COND directly can turn + previously multiply used SSA_NAMEs into single use SSA_NAMEs, so + it needs to be done in a separate loop afterwards. */ + for (bb = last_bb, idx = 0; ; bb = single_pred (bb), idx++) { - if (bbinfo[idx].first_idx < bbinfo[idx].last_idx) + if (bbinfo[idx].first_idx < bbinfo[idx].last_idx + && bbinfo[idx].op != NULL_TREE) { - gimple stmt = last_stmt (bb); tree new_op; - if (bbinfo[idx].op == NULL_TREE) - { - if (ops[bbinfo[idx].first_idx]->op != NULL_TREE) - { - if (integer_zerop (ops[bbinfo[idx].first_idx]->op)) - gimple_cond_make_false (stmt); - else if (integer_onep (ops[bbinfo[idx].first_idx]->op)) - gimple_cond_make_true (stmt); - else - { - gimple_cond_set_code (stmt, NE_EXPR); - gimple_cond_set_lhs (stmt, - ops[bbinfo[idx].first_idx]->op); - gimple_cond_set_rhs (stmt, boolean_false_node); - } - update_stmt (stmt); - } - bbinfo[idx].op = new_op = boolean_false_node; - } - else - new_op = update_ops (bbinfo[idx].op, - (enum tree_code) - ops[bbinfo[idx].first_idx]->rank, - ops, &bbinfo[idx].first_idx, - loop_containing_stmt (stmt)); + stmt = last_stmt (bb); + new_op = update_ops (bbinfo[idx].op, + (enum tree_code) + ops[bbinfo[idx].first_idx]->rank, + ops, &bbinfo[idx].first_idx, + loop_containing_stmt (stmt)); if (new_op == NULL_TREE) { gcc_assert (bb == last_bb); @@ -2955,6 +2946,28 @@ maybe_optimize_range_tests (gimple stmt) } if (bb == first_bb) break; + } + for (bb = last_bb, idx = 0; ; bb = single_pred (bb), idx++) + { + if (bbinfo[idx].first_idx < bbinfo[idx].last_idx + && bbinfo[idx].op == NULL_TREE + && ops[bbinfo[idx].first_idx]->op != NULL_TREE) + { + stmt = last_stmt (bb); + if (integer_zerop (ops[bbinfo[idx].first_idx]->op)) + gimple_cond_make_false (stmt); + else if (integer_onep (ops[bbinfo[idx].first_idx]->op)) + gimple_cond_make_true (stmt); + else + { + gimple_cond_set_code (stmt, NE_EXPR); + gimple_cond_set_lhs (stmt, ops[bbinfo[idx].first_idx]->op); + gimple_cond_set_rhs (stmt, boolean_false_node); + } + update_stmt (stmt); + } + if (bb == first_bb) + break; } } bbinfo.release (); --- gcc/testsuite/gcc.c-torture/compile/pr58946.c.jj 2013-11-01 08:29:52.484276440 +0100 +++ gcc/testsuite/gcc.c-torture/compile/pr58946.c 2013-11-01 08:29:29.000000000 +0100 @@ -0,0 +1,20 @@ +/* PR tree-optimization/58946 */ + +int +foo (unsigned int c) +{ + unsigned int d, e, f; + if ((int) c < 0) + d = 0; + else + d = c; + if (d == 0) + e = __INT_MAX__ + 1U; + else + e = d; + if ((int) e < 0) + f = 0; + else + f = e; + return f; +}