From patchwork Tue Nov 29 16:33:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 700641 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 3tSpz73kn2z9t2C for ; Wed, 30 Nov 2016 03:34:10 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="vGB9qMUf"; 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:to:cc :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=rtvPmKQZJb24biWU9gUDP/M1AFbGhUgnZU1juyAdsOMwoaUgpV tk1GFHrZhIqctzShBd+26Jlqq8GS/WjBNlOxiku2ueYQtasIW/AwEdw1Lv22cfR3 JRH9R3LxParpxnU4NtIwdRBU+dpBGc8dhsbtjwifX6/Y9QkX70hxmrXjk= 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:to:cc :from:subject:message-id:date:mime-version:content-type; s= default; bh=d/9Us5XFCG28TOfuUL1ZY5/rx5I=; b=vGB9qMUfwt409we4VmeY Uq3v6+XfsXomiLLImSAiu7LLAxU3dCrdn1t7B+yzhjSF0shXWEC2p/CLagfx2Hw7 n6eCy4vpfNCBFPaaazkgH2gGQ8WmHU0ceIrjO9pdtNJV/vXDdunAHP9D7UtIalEX uvKQb95zc6CdNjkIpgYirLA= Received: (qmail 109402 invoked by alias); 29 Nov 2016 16:33:59 -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 109325 invoked by uid 89); 29 Nov 2016 16:33:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.8 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=inverse, auto_vec, H*M:4162, H*MI:4162 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; Tue, 29 Nov 2016 16:33:55 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 86A383D96A; Tue, 29 Nov 2016 16:33:54 +0000 (UTC) Received: from reynosa.quesejoda.com (vpn-54-177.rdu2.redhat.com [10.10.54.177]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uATGXp8q025258; Tue, 29 Nov 2016 11:33:52 -0500 To: gcc-patches , Jeff Law Cc: Christophe Lyon , Andreas Schwab From: Aldy Hernandez Subject: [PR middle-end/78566] Fix uninit regressions caused by previous -Wmaybe-uninit change Message-ID: <1684bcc0-a007-b052-4162-8ce54ae78c27@redhat.com> Date: Tue, 29 Nov 2016 11:33:51 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 This fixes the gcc.dg/uninit-pred-6* failures I seem to have caused on some non x86 platforms. Sorry for the delay. The problem is that my fix for PR61409 had the logic backwards. I was proving that all the uses of a PHI are invalidated by any one undefined PHI path, whereas what we want is to prove that EVERY uninitialized path is invalidated by some facor in the PHI use. The attached patch fixes this without causing any regressions on x86-64 Linux. I also verified that at least on [arm-none-linux-gnueabihf --with-cpu=cortex-a5 --with-fpu=vfpv3-d16-fp16], there are no gcc.dg/*uninit* regressions. There is still one regression at large involving a double free in PR78548 which I will look at next/independently. OK for trunk? Aldy commit 469f4c38a48bc284c268b40f5d5511f015844ea2 Author: Aldy Hernandez Date: Tue Nov 29 05:59:53 2016 -0500 PR middle-end/78566 * tree-ssa-uninit.c (can_one_predicate_be_invalidated_p): Change argument type to a pred_chain. (can_chain_union_be_invalidated_p): Use pred_chain instead of a worklist. (flatten_out_predicate_chains): Remove. (uninit_uses_cannot_happen): Rename from uninit_ops_invalidate_phi_use. Change logic so that we are checking that the PHI use will invalidate _ALL_ possibly uninitialized operands. (is_use_properly_guarded): Rename call to uninit_ops_invalidate_phi_use into uninit_uses_cannot_happen. diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c index 4557403..a648995 100644 --- a/gcc/tree-ssa-uninit.c +++ b/gcc/tree-ssa-uninit.c @@ -2155,115 +2155,66 @@ normalize_preds (pred_chain_union preds, gimple *use_or_def, bool is_use) static bool can_one_predicate_be_invalidated_p (pred_info predicate, - vec worklist) + pred_chain use_guard) { - for (size_t i = 0; i < worklist.length (); ++i) + for (size_t i = 0; i < use_guard.length (); ++i) { - pred_info *p = worklist[i]; - /* NOTE: This is a very simple check, and only understands an exact opposite. So, [i == 0] is currently only invalidated by [.NOT. i == 0] or [i != 0]. Ideally we should also invalidate with say [i > 5] or [i == 8]. There is certainly room for improvement here. */ - if (pred_neg_p (predicate, *p)) + if (pred_neg_p (predicate, use_guard[i])) return true; } return false; } -/* Return TRUE if all USE_PREDS can be invalidated by some predicate - in WORKLIST. */ +/* Return TRUE if all predicates in UNINIT_PRED are invalidated by + USE_GUARD being true. */ static bool -can_chain_union_be_invalidated_p (pred_chain_union use_preds, - vec worklist) +can_chain_union_be_invalidated_p (pred_chain_union uninit_pred, + pred_chain use_guard) { - /* Remember: - PRED_CHAIN_UNION = PRED_CHAIN1 || PRED_CHAIN2 || PRED_CHAIN3 - PRED_CHAIN = PRED_INFO1 && PRED_INFO2 && PRED_INFO3, etc. - - We need to invalidate the entire PRED_CHAIN_UNION, which means, - invalidating every PRED_CHAIN in this union. But to invalidate - an individual PRED_CHAIN, all we need to invalidate is _any_ one - PRED_INFO, by boolean algebra !PRED_INFO1 || !PRED_INFO2... */ - for (size_t i = 0; i < use_preds.length (); ++i) + if (uninit_pred.is_empty ()) + return false; + for (size_t i = 0; i < uninit_pred.length (); ++i) { - pred_chain c = use_preds[i]; - bool entire_pred_chain_invalidated = false; + pred_chain c = uninit_pred[i]; for (size_t j = 0; j < c.length (); ++j) - if (can_one_predicate_be_invalidated_p (c[j], worklist)) - { - entire_pred_chain_invalidated = true; - break; - } - if (!entire_pred_chain_invalidated) - return false; + if (!can_one_predicate_be_invalidated_p (c[j], use_guard)) + return false; } return true; } -/* Flatten out all the factors in all the pred_chain_union's in PREDS - into a WORKLIST of individual PRED_INFO's. +/* Return TRUE if none of the uninitialized operands in UNINT_OPNDS + can actually happen if we arrived at a use for PHI. - N is the number of pred_chain_union's in PREDS. + PHI_USE_GUARDS are the guard conditions for the use of the PHI. */ - Since we are interested in the inverse of the PRED_CHAIN's, by - boolean algebra, an inverse turns those PRED_CHAINS into unions, - which means we can flatten all the factors out for easy access. */ - -static void -flatten_out_predicate_chains (pred_chain_union preds[], size_t n, - vec *worklist) +static bool +uninit_uses_cannot_happen (gphi *phi, unsigned uninit_opnds, + pred_chain_union phi_use_guards) { - for (size_t i = 0; i < n; ++i) - { - pred_chain_union u = preds[i]; - for (size_t j = 0; j < u.length (); ++j) - { - pred_chain c = u[j]; - for (size_t k = 0; k < c.length (); ++k) - worklist->safe_push (&c[k]); - } - } -} - -/* Return TRUE if executing the path to some uninitialized operands in - a PHI will invalidate the use of the PHI result later on. - - UNINIT_OPNDS is a bit vector specifying which PHI arguments have - arguments which are considered uninitialized. - - USE_PREDS is the pred_chain_union specifying the guard conditions - for the use of the PHI result. - - What we want to do is disprove each of the guards in the factors of - the USE_PREDS. So if we have: - - # USE_PREDS guards of: - # 1. i > 5 && i < 100 - # 2. j > 10 && j < 88 - - Then proving that the control dependenies for the UNINIT_OPNDS are: - - # [i <= 5] - # .OR. [i >= 100] - # + unsigned phi_args = gimple_phi_num_args (phi); + if (phi_args > max_phi_args) + return false; - ...we can prove that the 1st guard above in USE_PREDS is invalid. - Similarly for the 2nd guard. We return TRUE if we can disprove - both of the guards in USE_PREDS above. */ + /* PHI_USE_GUARDS are OR'ed together. If we have more than one + possible guard, there's no way of knowing which guard was true. + Since we need to be absolutely sure that the uninitialized + operands will be invalidated, bail. */ + if (phi_use_guards.length () != 1) + return false; -static bool -uninit_ops_invalidate_phi_use (gphi *phi, unsigned uninit_opnds, - pred_chain_union use_preds) -{ /* Look for the control dependencies of all the uninitialized - operands and build predicates describing them. */ + operands and build guard predicates describing them. */ unsigned i; pred_chain_union uninit_preds[max_phi_args]; - memset (uninit_preds, 0, sizeof (pred_chain_union) * max_phi_args); - for (i = 0; i < MIN (max_phi_args, gimple_phi_num_args (phi)); i++) + memset (uninit_preds, 0, sizeof (pred_chain_union) * phi_args); + for (i = 0; i < phi_args; ++i) { if (!MASK_TEST_BIT (uninit_opnds, i)) continue; @@ -2274,32 +2225,27 @@ uninit_ops_invalidate_phi_use (gphi *phi, unsigned uninit_opnds, size_t num_chains = 0; int num_calls = 0; - /* Build the control dependency chain for `i'... */ - if (compute_control_dep_chain (find_dom (e->src), - e->src, - dep_chains, - &num_chains, - &cur_chain, - &num_calls)) - { - /* ...and convert it into a set of predicates. */ - convert_control_dep_chain_into_preds (dep_chains, num_chains, - &uninit_preds[i]); - for (size_t j = 0; j < num_chains; ++j) - dep_chains[j].release (); - simplify_preds (&uninit_preds[i], NULL, false); - uninit_preds[i] - = normalize_preds (uninit_preds[i], NULL, false); - } + /* Build the control dependency chain for uninit operand `i'... */ + if (!compute_control_dep_chain (find_dom (e->src), + e->src, dep_chains, &num_chains, + &cur_chain, &num_calls)) + return false; + /* ...and convert it into a set of predicates. */ + convert_control_dep_chain_into_preds (dep_chains, num_chains, + &uninit_preds[i]); + for (size_t j = 0; j < num_chains; ++j) + dep_chains[j].release (); + simplify_preds (&uninit_preds[i], NULL, false); + uninit_preds[i] + = normalize_preds (uninit_preds[i], NULL, false); + + /* Can the guard for this uninitialized operand be invalidated + by the PHI use? */ + if (!can_chain_union_be_invalidated_p (uninit_preds[i], + phi_use_guards[0])) + return false; } - - /* Munge all the predicates into one worklist, and see if we can - invalidate all the chains in USE_PREDs with the predicates in - WORKLIST. */ - auto_vec worklist; - flatten_out_predicate_chains (uninit_preds, i, &worklist); - bool ret = can_chain_union_be_invalidated_p (use_preds, worklist); - return ret; + return true; } /* Computes the predicates that guard the use and checks @@ -2361,8 +2307,8 @@ is_use_properly_guarded (gimple *use_stmt, for UNINIT_OPNDS are true, that the control dependencies for USE_STMT can never be true. */ if (!is_properly_guarded) - is_properly_guarded |= uninit_ops_invalidate_phi_use (phi, uninit_opnds, - preds); + is_properly_guarded |= uninit_uses_cannot_happen (phi, uninit_opnds, + preds); if (is_properly_guarded) {