From patchwork Sun May 17 19:11:35 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marc Glisse X-Patchwork-Id: 473214 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 7940514029C for ; Mon, 18 May 2015 05:11:49 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=XV5/7km1; 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:subject:message-id:mime-version:content-type; q=dns; s= default; b=ltBvm9kw+0nWyuc4uj9G22b9Kn5bDNqaZ3Jd1VXiiqvX9U5imXBW8 73ex4JLvvSpMpO/EkI0K5VgGJkLd5HOd3l86wjXG3l15hdkPqUIVKZkBDfmIPX3i nHudpjtk5/l4SdgfaPiGssYuY6iCTjYUKPFX9qO+hY0rouE1J26JDw= 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:subject:message-id:mime-version:content-type; s= default; bh=MLfenn53haMKIXuXdYi7yDg8vxE=; b=XV5/7km1v6NIqDRif2Te X6Ynury+Y321kdxk2d3S335VQoz1wCTw6k67txmT5GaNjyVklHTBMkF37FYJI1Vr cWOqMQUKXSTLWp5P7LBSeeEkDK0eBpsHcAWKo9YW7YooXqiG21ENXHwccKQjHNbx hVKnX07DVWJrukiOgym/N58= Received: (qmail 57871 invoked by alias); 17 May 2015 19:11:41 -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 57862 invoked by uid 89); 17 May 2015 19:11:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, KAM_ASCII_DIVIDERS, KAM_LAZY_DOMAIN_SECURITY, T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mail2-relais-roc.national.inria.fr Received: from mail2-relais-roc.national.inria.fr (HELO mail2-relais-roc.national.inria.fr) (192.134.164.83) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Sun, 17 May 2015 19:11:39 +0000 Received: from stedding.saclay.inria.fr (HELO stedding) ([193.55.250.194]) by mail2-relais-roc.national.inria.fr with ESMTP/TLS/AES128-SHA; 17 May 2015 21:11:35 +0200 Received: from glisse (helo=localhost) by stedding with local-esmtp (Exim 4.85) (envelope-from ) id 1Yu3yJ-0005es-Hc for gcc-patches@gcc.gnu.org; Sun, 17 May 2015 21:11:35 +0200 Date: Sun, 17 May 2015 21:11:35 +0200 (CEST) From: Marc Glisse To: gcc-patches@gcc.gnu.org Subject: PRE and uninitialized variables Message-ID: User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 Hello, first, this patch is not ready (it doesn't even bootstrap), I am posting it for comments. The idea is, when we do value numbering, while looking for the current value of a local variable, we may hit a clobber or reach the beginning of the function. In both cases, it seems to me that we could use a default definition as the value of the variable. This later allows the uninit pass to warn about the use of an uninitialized or clobbered variable (some passes can also optimize the code better). The part about clobbers passed regtesting, but the part about reaching the entry of the function fails bootstrap with: Existing SSA name for symbol marked for renaming: __d_14(D) internal compiler error: SSA corruption when execute_update_addresses_taken is called after SRA. In the case I looked at, the new code was called on the lhs of an assignment (to check if both sides had the same value number). Creating new ssa_names in the middle of sccvn is probably a bad idea, although it doesn't help if I create them beforehand, maybe creating the default definition and leaving it unused means that no one feels responsible for cleaning it up? (TODO_update_ssa doesn't help) I am quite restrictive in the conditions for the code to apply: is_gimple_reg_type, useless_type_conversion_p (uh? I forgot that one in the "reached entry" case...), I am not sure what I can do if the local variable is an aggregate and we are reading one field, maybe create an artificial variable just for the purpose of using its default definition? Mostly, I would like to know if the approach makes sense. Is storing the default definition in SSA_VAL ok, or is there some way to use VN_TOP to mark undefinedness? Any pointer would be welcome... * tree-ssa-sccvn.c (vn_reference_lookup_2): Handle function entry. (vn_reference_lookup_3): Handle clobbers. (init_scc_vn): Default definitions are their own definition. PS: the testsuite for libgomp is looong, it almost doubles the time for the whole testsuite to complete on gcc112. It would be great if someone could split it into a few pieces that run in parallel. Index: gcc/testsuite/gcc.dg/uninit-clob.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-clob.c (revision 0) +++ gcc/testsuite/gcc.dg/uninit-clob.c (working copy) @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-options "-O -Wuninitialized" } */ + +int *p; + +int f(){ + { + int q = 42; + p = &q; + } + return *p; /* { dg-warning "uninitialized" "warning" } */ +} + Index: gcc/testsuite/gcc.dg/uninit-sccvn.c =================================================================== --- gcc/testsuite/gcc.dg/uninit-sccvn.c (revision 0) +++ gcc/testsuite/gcc.dg/uninit-sccvn.c (working copy) @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-O -Wuninitialized" } */ + +int g, h; +void *p; +int f(int x){ + int a; + g = 42; + h = a; + p = &a; + return h; /* { dg-warning "uninitialized" "warning" } */ +} Index: gcc/tree-ssa-sccvn.c =================================================================== --- gcc/tree-ssa-sccvn.c (revision 223269) +++ gcc/tree-ssa-sccvn.c (working copy) @@ -1553,26 +1553,33 @@ vn_reference_lookup_1 (vn_reference_t vr *vnresult = (vn_reference_t)*slot; return ((vn_reference_t)*slot)->result; } return NULL_TREE; } static tree *last_vuse_ptr; static vn_lookup_kind vn_walk_kind; static vn_lookup_kind default_vn_walk_kind; +static vn_reference_t +vn_reference_lookup_or_insert_for_pieces (tree vuse, + alias_set_type set, + tree type, + vec operands, + tree value); /* Callback for walk_non_aliased_vuses. Adjusts the vn_reference_t VR_ with the current VUSE and performs the expression lookup. */ static void * -vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, +vn_reference_lookup_2 (ao_ref *ref, tree vuse, unsigned int cnt, void *vr_) { vn_reference_t vr = (vn_reference_t)vr_; vn_reference_s **slot; hashval_t hash; /* This bounds the stmt walks we perform on reference lookups to O(1) instead of O(N) where N is the number of dominating stores. */ if (cnt > (unsigned) PARAM_VALUE (PARAM_SCCVN_MAX_ALIAS_QUERIES_PER_ACCESS)) @@ -1587,20 +1594,39 @@ vn_reference_lookup_2 (ao_ref *op ATTRIB vr->vuse = vuse_ssa_val (vuse); if (vr->vuse) vr->hashcode = vr->hashcode + SSA_NAME_VERSION (vr->vuse); hash = vr->hashcode; slot = current_info->references->find_slot_with_hash (vr, hash, NO_INSERT); if (!slot && current_info == optimistic_info) slot = valid_info->references->find_slot_with_hash (vr, hash, NO_INSERT); if (slot) return *slot; + if (gimple_nop_p (SSA_NAME_DEF_STMT (vuse))) + { + tree base = ao_ref_base (ref); + if (TREE_CODE (base) == VAR_DECL + && !is_global_var (base) + && is_gimple_reg_type (vr->type)) + { + tree val = ssa_default_def (cfun, base); + if (!val) + { + val = get_or_create_ssa_default_def (cfun, base); + VN_INFO_GET (val)->valnum = val; + VN_INFO (val)->expr = NULL_TREE; + VN_INFO (val)->value_id = 0; + } + return vn_reference_lookup_or_insert_for_pieces + (vuse, vr->set, vr->type, vr->operands, val); + } + } return NULL; } /* Lookup an existing or insert a new vn_reference entry into the value table for the VUSE, SET, TYPE, OPERANDS reference which has the value VALUE which is either a constant or an SSA name. */ static vn_reference_t vn_reference_lookup_or_insert_for_pieces (tree vuse, @@ -1712,21 +1738,40 @@ vn_reference_lookup_3 (ao_ref *ref, tree offset = ref->offset; maxsize = ref->max_size; /* If we cannot constrain the size of the reference we cannot test if anything kills it. */ if (maxsize == -1) return (void *)-1; /* We can't deduce anything useful from clobbers. */ if (gimple_clobber_p (def_stmt)) - return (void *)-1; + { + if (TREE_CODE (base) == VAR_DECL + && !is_global_var (base) + && operand_equal_p (base, gimple_assign_lhs (def_stmt), 0) + && is_gimple_reg_type (vr->type) + && useless_type_conversion_p (vr->type, TREE_TYPE (base))) + { + tree val = ssa_default_def (cfun, base); + if (!val) + { + val = get_or_create_ssa_default_def (cfun, base); + VN_INFO_GET (val)->valnum = val; + VN_INFO (val)->expr = NULL_TREE; + VN_INFO (val)->value_id = 0; + } + return vn_reference_lookup_or_insert_for_pieces + (vuse, vr->set, vr->type, vr->operands, val); + } + return (void *)-1; + } /* def_stmt may-defs *ref. See if we can derive a value for *ref from that definition. 1) Memset. */ if (is_gimple_reg_type (vr->type) && gimple_call_builtin_p (def_stmt, BUILT_IN_MEMSET) && integer_zerop (gimple_call_arg (def_stmt, 1)) && tree_fits_uhwi_p (gimple_call_arg (def_stmt, 2)) && TREE_CODE (gimple_call_arg (def_stmt, 0)) == ADDR_EXPR) { @@ -4190,21 +4235,22 @@ init_scc_vn (void) VN_TOP = create_tmp_var_raw (void_type_node, "vn_top"); /* Create the VN_INFO structures, and initialize value numbers to TOP. */ for (i = 0; i < num_ssa_names; i++) { tree name = ssa_name (i); if (name) { - VN_INFO_GET (name)->valnum = VN_TOP; + VN_INFO_GET (name)->valnum = SSA_NAME_IS_DEFAULT_DEF (name) ? name + : VN_TOP; VN_INFO (name)->expr = NULL_TREE; VN_INFO (name)->value_id = 0; } } renumber_gimple_stmt_uids (); /* Create the valid and optimistic value numbering tables. */ valid_info = XCNEW (struct vn_tables_s); allocate_vn_table (valid_info);