From patchwork Thu Dec 3 20:58:38 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 552452 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.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 582201402A1 for ; Fri, 4 Dec 2015 07:58:52 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=TL46Uc0f; dkim-atps=neutral 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=tw03lpi72/J+PnTNmSw4/sPqmQEiI b4I/JGErTABa30u9e01FTvnJRFHBwHCc0sTxGOaLHvFDNx3qRI6IY1fYuH+5zqTB 5i3gGMf4ciLkr57KSpNb+j2xRZ0Z1rkz3HFrUqCGBexWzpR+WRmb4U0MfwOx1Cg/ kL8KcdsePGQxFI= 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=vSNeFMWDpyqPJu1TgY0DyAMFNvA=; b=TL4 6Uc0fLqpR4hM2mbiUV06u62pObe8Cv+huAfu/NTwpsOY4yGiDbOHCv7GHdSJ3DWA PH13rCKAHJQTwEkkFyTldILnSeeaV2aCv0TjC49U9G0BBEL695CwjlE8DXRjkhYP IeS0yRT5CU/qYE29C1bP5VSe8oc57VnnZqX7S5ro= Received: (qmail 120537 invoked by alias); 3 Dec 2015 20:58:46 -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 120511 invoked by uid 89); 3 Dec 2015 20:58:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, T_RP_MATCHES_RCVD 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 03 Dec 2015 20:58:44 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id E6E19C0D7F24; Thu, 3 Dec 2015 20:58:42 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-116-34.ams2.redhat.com [10.36.116.34]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tB3KwfV8032560 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 3 Dec 2015 15:58:42 -0500 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id tB3KwdTv004705; Thu, 3 Dec 2015 21:58:39 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id tB3Kwc9R004656; Thu, 3 Dec 2015 21:58:38 +0100 Date: Thu, 3 Dec 2015 21:58:38 +0100 From: Jakub Jelinek To: Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] Fix reassoc range test vs. value ranges (PR tree-optimization/68671) Message-ID: <20151203205838.GI5675@tucnak.redhat.com> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes Hi! As mentioned in the PR, maybe_optimize_range_tests considers basic blocks with not just the final GIMPLE_COND (or for last_bb store feeding into PHI), but also assign stmts that don't trap, don't have side-effects and where the SSA_NAMEs they set are used only in their own bb. Now, if we decide to optimize some range test, we can change some conditions on previous bbs and that means we could execute some basic blocks that wouldn't be executed in the original program. As the stmts don't set anything used in other bbs, they are most likely dead after the optimization, but the problem on the testcase is that because of the condition changes in previous bb we end up with incorrect value range for some SSA_NAME(s). That can result in the miscompilation of the testcase on certain targets. Fixed by resetting the value range info of such SSA_NAMEs. I believe it shouldn't be a big deal, they will be mostly dead anyway. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2015-12-03 Jakub Jelinek PR tree-optimization/68671 * tree-ssa-reassoc.c (maybe_optimize_range_tests): For basic blocks starting with the successor of first bb we've modified and ending with last_bb, reset value ranges of all integral SSA_NAMEs set in those basic blocks. * gcc.dg/pr68671.c: New test. Jakub --- gcc/tree-ssa-reassoc.c.jj 2015-11-18 11:22:51.000000000 +0100 +++ gcc/tree-ssa-reassoc.c 2015-12-03 18:12:08.915210122 +0100 @@ -3204,7 +3204,7 @@ maybe_optimize_range_tests (gimple *stmt any_changes = optimize_range_tests (ERROR_MARK, &ops); if (any_changes) { - unsigned int idx; + unsigned int idx, max_idx = 0; /* 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 @@ -3220,6 +3220,7 @@ maybe_optimize_range_tests (gimple *stmt { tree new_op; + max_idx = idx; stmt = last_stmt (bb); new_op = update_ops (bbinfo[idx].op, (enum tree_code) @@ -3289,6 +3290,10 @@ maybe_optimize_range_tests (gimple *stmt && ops[bbinfo[idx].first_idx]->op != NULL_TREE) { gcond *cond_stmt = as_a (last_stmt (bb)); + + if (idx > max_idx) + max_idx = idx; + if (integer_zerop (ops[bbinfo[idx].first_idx]->op)) gimple_cond_make_false (cond_stmt); else if (integer_onep (ops[bbinfo[idx].first_idx]->op)) @@ -3305,6 +3310,30 @@ maybe_optimize_range_tests (gimple *stmt if (bb == first_bb) break; } + + /* The above changes could result in basic blocks after the first + modified one, up to and including last_bb, to be executed even if + they would not be in the original program. If the value ranges of + assignment lhs' in those bbs were dependent on the conditions + guarding those basic blocks which now can change, the VRs might + be incorrect. As no_side_effect_bb should ensure those SSA_NAMEs + are only used within the same bb, it should be not a big deal if + we just reset all the VRs in those bbs. See PR68671. */ + for (bb = last_bb, idx = 0; idx < max_idx; bb = single_pred (bb), idx++) + { + gimple_stmt_iterator gsi; + for (gsi = gsi_last_bb (bb); !gsi_end_p (gsi); gsi_prev (&gsi)) + { + gimple *g = gsi_stmt (gsi); + if (!is_gimple_assign (g)) + continue; + tree lhs = gimple_assign_lhs (g); + if (TREE_CODE (lhs) != SSA_NAME) + continue; + if (INTEGRAL_TYPE_P (TREE_TYPE (lhs))) + SSA_NAME_RANGE_INFO (lhs) = NULL; + } + } } } --- gcc/testsuite/gcc.dg/pr68671.c.jj 2015-12-03 18:19:24.769104484 +0100 +++ gcc/testsuite/gcc.dg/pr68671.c 2015-12-03 18:19:07.000000000 +0100 @@ -0,0 +1,23 @@ +/* PR tree-optimization/68671 */ +/* { dg-do run } */ +/* { dg-options " -O2 -fno-tree-dce" } */ + +volatile int a = -1; +volatile int b; + +static inline int +fn1 (signed char p1, int p2) +{ + return (p1 < 0) || (p1 > (1 >> p2)) ? 0 : (p1 << 1); +} + +int +main () +{ + signed char c = a; + b = fn1 (c, 1); + c = ((128 | c) < 0 ? 1 : 0); + if (c != 1) + __builtin_abort (); + return 0; +}