From patchwork Wed May 23 09:27:21 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandre Oliva X-Patchwork-Id: 160904 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]) by ozlabs.org (Postfix) with SMTP id 54CC1B6FBA for ; Wed, 23 May 2012 19:27:56 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1338370076; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Received:From:To:Subject:Date:Message-ID: User-Agent:MIME-Version:Content-Type:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=pqOtVwn75A5nQ/gMeYL5MYO+UCE=; b=dysZC3uBPCDI+WT OyoeTp5R210UCEAaSKx/zV+EOum52SJnyc+AdZ7eedn2WEs71pdRWaurVPiiWaRT y4mjJZsTKg3PDQwthY0gNhGWLS0CutqJRm1YA4G6Jwxnl97NvjEB5lNaCpQqlyBs cIrqioEWO/YXpk3lgRXWsZW2KCsw= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Received:Received:From:To:Subject:Date:Message-ID:User-Agent:MIME-Version:Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=uSlsN7fqhIDlfErxda1FXNBctQC7LN+C2VvdbRlGN/+sa+H3ef7w+pqq9KmzSN 81RlxfpUJ6u1LVMt4vG2ctTgoWv3CiMX31UC4rNoM6abOKSCj6l+NZFu6suxuPsY aNNDGOjFGusuEEFik5YFu/7vY7h2iQp2xbXFvVpfbmXqU=; Received: (qmail 5075 invoked by alias); 23 May 2012 09:27:49 -0000 Received: (qmail 5053 invoked by uid 22791); 23 May 2012 09:27:46 -0000 X-SWARE-Spam-Status: No, hits=-5.6 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, SPF_HELO_PASS, T_RP_MATCHES_RCVD, T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 23 May 2012 09:27:28 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q4N9RRuE026475 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 23 May 2012 05:27:27 -0400 Received: from freie.oliva.athome.lsd.ic.unicamp.br (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q4N9RPp5007651 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Wed, 23 May 2012 05:27:27 -0400 Received: from livre.localdomain (livre-to-gw.oliva.athome.lsd.ic.unicamp.br [172.31.160.19]) by freie.oliva.athome.lsd.ic.unicamp.br (8.14.5/8.14.5) with ESMTP id q4N9RNd0006267 for ; Wed, 23 May 2012 06:27:23 -0300 Received: from livre.localdomain (aoliva@localhost.localdomain [127.0.0.1]) by livre.localdomain (8.14.3/8.14.3/Debian-5+lenny1) with ESMTP id q4N9RMHo001137; Wed, 23 May 2012 06:27:22 -0300 Received: (from aoliva@localhost) by livre.localdomain (8.14.3/8.14.3/Submit) id q4N9RMHC001136; Wed, 23 May 2012 06:27:22 -0300 From: Alexandre Oliva To: gcc-patches@gcc.gnu.org Subject: [PR49888, VTA] don't keep VALUEs bound to modified MEMs Date: Wed, 23 May 2012 06:27:21 -0300 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 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 Nothing in var-tracking was prepared to drop MEMs from bindings upon writes that might modify the MEM contents. That was correct, back when it was variables (rather than values) that were bound to MEMs, and we wanted the binding to remain even if the value stored in the variables changed. VTA changed that for gimple regs: we now bind variables to values, and values to locations that hold that value, so we must unbind them when the value stored in the MEM location changes, just like we already did for REGs. cselib might offer us some help to that end, but only within individual basic blocks. That's not quite enough: even a trivial testcase, as provided in the bug report, was enough to defeat the first approach I tried, of emitting MOps to unbind values from MEMs when cselib noticed the MEM was modified. Even in that trivial testcase, we wouldn't always unbind the incoming argument from the MEM, because it would be recorded using a different expression which we could only globally recognize as equivalent. And that didn't even take potential aliasing into account! In order to fix this debug info correctness bug, I ended up going with the following approach: after every MEM write, we go through all MEMs in the dataflow set binding table and unbind VALUEs bound to possibly-aliased MEMs. I'm sure there are cases in which this may remove perfectly valid location information, but getting the compiler to figure out they are indeed valid is impossible in some cases (eg incoming pointers known by the caller to not alias), and very, very expensive in others (eg inferring the lack of overlap by following equivalence chains). In the end, this patch didn't cause much loss of debug information on x86_64, but it does significantly reduce total coverage on i686 for libstdc++, libgcc and cc1plus. There was a also a significant drop in call_site_value and entry_value expressions, on both x86_64 and i686, slightly more pronounced on the latter. Unfortunately, I couldn't quantify how much of this reduction was because we of incorrect location information we generated before, and how much is because of overconservative decisions about potential aliasing. This was all kind of expected. What did surprise me was that this didn't have any measurable impact on bootstrap time. What surprised me further was that testing this patch confirmed the boostrap speedup of 1.08 I had observed with the proposed patch for PR47624, that this patch depends on. This patch was regstrapped along with the patch for PR47624 (ping ) on x86_64-linux-gnu and i686-linux-gnu. Ok to install? for gcc/ChangeLog from Alexandre Oliva PR debug/49888 * var-tracking.c: Include alias.h. (overlapping_mems): New struct. (drop_overlapping_mem_locs): New. (clobber_overlapping_mems): New. (var_mem_delete_and_set, var_mem_delete): Call it. (val_bind): Likewise, but only if modified. (compute_bb_dataflow, emit_notes_in_bb): Call it on MEMs. * Makefile.in (var-tracking.o): Depend in $(ALIAS_H). for gcc/testsuite/ChangeLog from Alexandre Oliva PR debug/49888 * gcc.dg/guality/pr49888.c: New. Index: gcc/var-tracking.c =================================================================== --- gcc/var-tracking.c.orig 2012-05-16 14:58:11.000000000 -0300 +++ gcc/var-tracking.c 2012-05-21 02:24:57.000000000 -0300 @@ -116,6 +116,7 @@ #include "pointer-set.h" #include "recog.h" #include "tm_p.h" +#include "alias.h" /* var-tracking.c assumes that tree code with the same value as VALUE rtx code has no chance to appear in REG_EXPR/MEM_EXPRs and isn't a decl. @@ -1955,6 +1956,106 @@ var_regno_delete (dataflow_set *set, int *reg = NULL; } +/* Hold parameters for the hashtab traversal function + drop_overlapping_mem_locs, see below. */ + +struct overlapping_mems +{ + dataflow_set *set; + rtx loc; +}; + +/* Remove all MEMs that overlap with COMS->LOC from the location list + of a hash table entry for a value. */ + +static int +drop_overlapping_mem_locs (void **slot, void *data) +{ + struct overlapping_mems *coms = (struct overlapping_mems *)data; + dataflow_set *set = coms->set; + rtx mloc = coms->loc; + variable var = (variable) *slot; + + if (var->onepart == ONEPART_VALUE) + { + location_chain loc, *locp; + bool changed = false; + rtx cur_loc; + + gcc_assert (var->n_var_parts == 1); + + if (shared_var_p (var, set->vars)) + { + for (loc = var->var_part[0].loc_chain; loc; loc = loc->next) + if (GET_CODE (loc->loc) == MEM + && !nonoverlapping_memrefs_p (loc->loc, mloc, false)) + break; + + if (!loc) + return 1; + + slot = unshare_variable (set, slot, var, VAR_INIT_STATUS_UNKNOWN); + var = (variable)*slot; + gcc_assert (var->n_var_parts == 1); + } + + if (VAR_LOC_1PAUX (var)) + cur_loc = VAR_LOC_FROM (var); + else + cur_loc = var->var_part[0].cur_loc; + + for (locp = &var->var_part[0].loc_chain, loc = *locp; + loc; loc = *locp) + { + if (GET_CODE (loc->loc) != MEM + || nonoverlapping_memrefs_p (loc->loc, mloc, false)) + { + locp = &loc->next; + continue; + } + + *locp = loc->next; + /* If we have deleted the location which was last emitted + we have to emit new location so add the variable to set + of changed variables. */ + if (cur_loc == loc->loc) + { + changed = true; + var->var_part[0].cur_loc = NULL; + if (VAR_LOC_1PAUX (var)) + VAR_LOC_FROM (var) = NULL; + } + pool_free (loc_chain_pool, loc); + } + + if (!var->var_part[0].loc_chain) + { + var->n_var_parts--; + changed = true; + } + if (changed) + variable_was_changed (var, set); + } + + return 1; +} + +/* Remove from SET all VALUE bindings to MEMs that overlap with LOC. */ + +static void +clobber_overlapping_mems (dataflow_set *set, rtx loc) +{ + struct overlapping_mems coms; + + coms.set = set; + coms.loc = loc; + + set->traversed_vars = set->vars; + htab_traverse (shared_hash_htab (set->vars), + drop_overlapping_mem_locs, &coms); + set->traversed_vars = NULL; +} + /* Set the location of DV, OFFSET as the MEM LOC. */ static void @@ -1997,6 +2098,7 @@ var_mem_delete_and_set (dataflow_set *se tree decl = MEM_EXPR (loc); HOST_WIDE_INT offset = INT_MEM_OFFSET (loc); + clobber_overlapping_mems (set, loc); decl = var_debug_decl (decl); if (initialized == VAR_INIT_STATUS_UNKNOWN) @@ -2017,6 +2119,7 @@ var_mem_delete (dataflow_set *set, rtx l tree decl = MEM_EXPR (loc); HOST_WIDE_INT offset = INT_MEM_OFFSET (loc); + clobber_overlapping_mems (set, loc); decl = var_debug_decl (decl); if (clobber) clobber_variable_part (set, NULL, dv_from_decl (decl), offset, NULL); @@ -2060,6 +2163,9 @@ val_bind (dataflow_set *set, rtx val, rt { struct elt_loc_list *l = CSELIB_VAL_PTR (val)->locs; + if (modified) + clobber_overlapping_mems (set, loc); + if (l && GET_CODE (l->loc) == VALUE) l = canonical_cselib_val (CSELIB_VAL_PTR (l->loc))->locs; @@ -6372,6 +6478,8 @@ compute_bb_dataflow (basic_block bb) } else if (REG_P (uloc)) var_regno_delete (out, REGNO (uloc)); + else if (MEM_P (uloc)) + clobber_overlapping_mems (out, uloc); val_store (out, val, dstv, insn, true); } @@ -8871,6 +8979,8 @@ emit_notes_in_bb (basic_block bb, datafl } else if (REG_P (uloc)) var_regno_delete (set, REGNO (uloc)); + else if (MEM_P (uloc)) + clobber_overlapping_mems (set, uloc); val_store (set, val, dstv, insn, true); Index: gcc/Makefile.in =================================================================== --- gcc/Makefile.in.orig 2012-05-21 02:21:12.424703696 -0300 +++ gcc/Makefile.in 2012-05-21 02:22:26.000000000 -0300 @@ -3129,8 +3129,8 @@ var-tracking.o : var-tracking.c $(CONFIG $(RTL_H) $(TREE_H) hard-reg-set.h insn-config.h reload.h $(FLAGS_H) \ $(BASIC_BLOCK_H) output.h sbitmap.h alloc-pool.h $(FIBHEAP_H) $(HASHTAB_H) \ $(REGS_H) $(EXPR_H) $(TIMEVAR_H) $(TREE_PASS_H) $(TREE_FLOW_H) \ - cselib.h $(TARGET_H) $(DIAGNOSTIC_CORE_H) $(PARAMS_H) $(DIAGNOSTIC_H) pointer-set.h \ - $(RECOG_H) $(TM_P_H) tree-pretty-print.h + cselib.h $(TARGET_H) $(DIAGNOSTIC_CORE_H) $(PARAMS_H) $(DIAGNOSTIC_H) \ + pointer-set.h $(RECOG_H) $(TM_P_H) tree-pretty-print.h $(ALIAS_H) profile.o : profile.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \ $(TREE_H) $(FLAGS_H) output.h $(REGS_H) $(EXPR_H) $(FUNCTION_H) $(BASIC_BLOCK_H) \ $(DIAGNOSTIC_CORE_H) $(COVERAGE_H) $(TREE_FLOW_H) value-prof.h cfghooks.h \ Index: gcc/testsuite/gcc.dg/guality/pr49888.c =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gcc/testsuite/gcc.dg/guality/pr49888.c 2012-05-16 14:58:12.000000000 -0300 @@ -0,0 +1,25 @@ +/* PR debug/49888 */ +/* { dg-do run } */ +/* { dg-options "-g" } */ + +static int v; + +static void __attribute__((noinline, noclone)) +f (int *p) +{ + int c = *p; + v = c; + *p = 1; /* { dg-final { gdb-test 12 "c" "0" } } */ + /* c may very well be optimized out at this point, so we test !c, + which will evaluate to the expected value. We just want to make + sure it doesn't remain bound to *p as it did before, in which + case !c would evaluate to 0. */ + v = 0; /* { dg-final { gdb-test 17 "!c" "1" } } */ +} +int +main () +{ + int a = 0; + f (&a); + return 0; +}