From patchwork Sat Jan 9 04:08:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandre Oliva X-Patchwork-Id: 565130 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 5DE4214031D for ; Sat, 9 Jan 2016 15:09:29 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=KjgWPZgn; 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:from :to:subject:date:message-id:mime-version:content-type; q=dns; s= default; b=F7K2sZDGvwNRPempbQJxQzBXDnuldHdyHt0haDjlcBWn0bic0rCi/ CbC1YWYtnDwESK1Qn7lXM+64bhBEeOiEpuMMMSNrrA3m2RP/T/vLSpmRjk1ai5m2 CGMM+A4KKJ7IThgsx0BLUlAiX+wCI8WA1an4qQ86xCfJbpfdo6HCzM= 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:from :to:subject:date:message-id:mime-version:content-type; s= default; bh=Pz2/Aye+aosCZQIBoju+Gbb5228=; b=KjgWPZgnluQNWUvPBJi0 hgH3bcRTYuiisOET47Y+PqhjA+wcrWAgY3b/055D6nowkpa39Di8iuv/cGOrKu9C yeWkDiUHS2gKjwz5Jq3C255TfP4kEdIcbN3CG5uERW5/PCi3fxW9lYI81OLnUSoG uBhCwCZnwCzKUvtXM006Kec= Received: (qmail 110159 invoked by alias); 9 Jan 2016 04:09:21 -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 110135 invoked by uid 89); 9 Jan 2016 04:09:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_20, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Latin, brasil, Brasil, free! 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; Sat, 09 Jan 2016 04:09:16 +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 (Postfix) with ESMTPS id 374D48535A for ; Sat, 9 Jan 2016 04:09:14 +0000 (UTC) Received: from freie.home (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 u09498fh032765 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO) for ; Fri, 8 Jan 2016 23:09:12 -0500 Received: from livre.home (livre.home [172.31.160.2]) by freie.home (8.15.2/8.15.2) with ESMTP id u0948paX013149; Sat, 9 Jan 2016 02:08:53 -0200 From: Alexandre Oliva To: gcc-patches@gcc.gnu.org Subject: [PR69123] fix VTA dataflow oscillation Date: Sat, 09 Jan 2016 02:08:51 -0200 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Here are two patches related with PR69123, an infinite dataflow loop in VTA. The first non-comment hunk in var-tracking.c:drop_overlapping_mem_locs is what fixes the problem, but the other changes in the first patch fix similar problems that might cause other such oscillations. The second patch adds some more information to detailed vartrack dumps, avoiding short-circuiting of dataflow set compares and dumping added and removed locations for variables present in both sets. The patches are largely independent, but they were successfully regstrapped together on x86_64-linux-gnu and i686-linux-gnu. They were only compile-tested separately. Ok to install? [PR69123] fix handling of MEMs in VTA to avoid dataflow oscillation From: Alexandre Oliva The problem arises because we used to drop overwritten MEMs from loc lists of VALUEs, but not of other onepart variables, and it just so happens that, by doing so, block 6 in the testcase has no D#5 in its output in the first pass, because the MEM holding its (previous) value was correctly dropped from value 88:88, but gains it in the second pass because D#5 has the MEM location incoming directly in its loc list, rather than indirectly in a VALUE. This incorrect binding enables other blocks to believe they have a tentative binding for D#5 in some cycles, but others, still operating on the early conclusion, believe there isn't, and they oscillate from that. Since we check for escaping MEMs in clobbers, we won't lose anything relevant by dropping call-clobbered or overwritten MEMs in all onepart variables, and this ensures the loc intersection operation in onepart vars won't let a MEM through that wasn't present in earlier iterations. for gcc/ChangeLog PR bootstrap/69123 * var-tracking.c (drop_overlapping_mem_locs): Operate on all onepart vars. Fix typo in comment. Fix reversed condition in unshare test. (dataflow_set_remove_mem_locs): Operate on all onepart vars. for gcc/testsuite/ChangeLog PR bootstrap/69123 * gcc.dg/pr69123.c: New. --- gcc/testsuite/gcc.dg/pr69123.c | 95 ++++++++++++++++++++++++++++++++++++++++ gcc/var-tracking.c | 12 +++-- 2 files changed, 101 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/pr69123.c diff --git a/gcc/testsuite/gcc.dg/pr69123.c b/gcc/testsuite/gcc.dg/pr69123.c new file mode 100644 index 0000000..0546e20 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr69123.c @@ -0,0 +1,95 @@ +/* { dg-do compile } */ +/* { dg-options "-O3 -g" } */ + +/* This was reduced from gcc/tree-vect-slp.c by H.J.Lu. */ + +struct xxx_def; +typedef xxx_def *xxx; + +union rtxxx +{ + const char *rt_str; + xxx rt_xxx; +}; + +struct xxx_def { + union u { + rtxxx fld[1]; + } u; +}; + +extern xxx bar (void); +extern int foo1 (xxx); + +static inline xxx +foo2 (xxx arg0, xxx arg1) +{ + xxx rt; + rt = bar (); + (((rt)->u.fld[0]).rt_xxx) = arg0; + (((rt)->u.fld[1]).rt_xxx) = arg1; + return rt; +} + +static inline xxx +foo4 (const char *arg0 ) +{ + xxx rt; + rt = bar (); + (((rt)->u.fld[0]).rt_str) = arg0; + (((rt)->u.fld[1]).rt_xxx) = (xxx) 0; + return rt; +} + +extern xxx foo5 (long); + +struct address_cost_data +{ + unsigned costs[2][2][2][2]; +}; + +void +get_address_cost (address_cost_data *data) +{ + unsigned acost; + long i; + long rat, off = 0; + unsigned sym_p, var_p, off_p, rat_p; + xxx addr, base; + xxx reg0, reg1; + + reg1 = bar (); + addr = foo2 (reg1, (xxx) 0); + rat = 1; + acost = 0; + reg0 = bar (); + reg1 = bar (); + + for (i = 0; i < 16; i++) + { + sym_p = i & 1; + var_p = (i >> 1) & 1; + off_p = (i >> 2) & 1; + rat_p = (i >> 3) & 1; + + addr = reg0; + if (rat_p) + addr = foo2 (addr, foo5 (rat)) ; + + if (var_p) + addr = foo2 (addr, reg1); + + if (sym_p) + base = foo4 (""); + else if (off_p) + base = foo5 (off); + else + base = (xxx) 0; + + if (base) + addr = foo2 (addr, base); + + acost = foo1 (addr); + data->costs[sym_p][var_p][off_p][rat_p] = acost; + } +} diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c index 634ebe0..a8931f3 100644 --- a/gcc/var-tracking.c +++ b/gcc/var-tracking.c @@ -2224,7 +2224,7 @@ struct overlapping_mems }; /* Remove all MEMs that overlap with COMS->LOC from the location list - of a hash table entry for a value. COMS->ADDR must be a + of a hash table entry for a onepart variable. COMS->ADDR must be a canonicalized form of COMS->LOC's address, and COMS->LOC must be canonicalized itself. */ @@ -2235,7 +2235,7 @@ drop_overlapping_mem_locs (variable **slot, overlapping_mems *coms) rtx mloc = coms->loc, addr = coms->addr; variable *var = *slot; - if (var->onepart == ONEPART_VALUE) + if (var->onepart != NOT_ONEPART) { location_chain *loc, **locp; bool changed = false; @@ -4682,11 +4682,11 @@ dataflow_set_preserve_mem_locs (variable **slot, dataflow_set *set) { for (loc = var->var_part[0].loc_chain; loc; loc = loc->next) { - /* We want to remove dying MEMs that doesn't refer to DECL. */ + /* We want to remove dying MEMs that don't refer to DECL. */ if (GET_CODE (loc->loc) == MEM && (MEM_EXPR (loc->loc) != decl || INT_MEM_OFFSET (loc->loc) != 0) - && !mem_dies_at_call (loc->loc)) + && mem_dies_at_call (loc->loc)) break; /* We want to move here MEMs that do refer to DECL. */ else if (GET_CODE (loc->loc) == VALUE @@ -4769,14 +4769,14 @@ dataflow_set_preserve_mem_locs (variable **slot, dataflow_set *set) } /* Remove all MEMs from the location list of a hash table entry for a - value. */ + onepart variable. */ int dataflow_set_remove_mem_locs (variable **slot, dataflow_set *set) { variable *var = *slot; - if (var->onepart == ONEPART_VALUE) + if (var->onepart != NOT_ONEPART) { location_chain *loc, **locp; bool changed = false; [PR69123] make dataflow_set_different details more verbose From: Alexandre Oliva for gcc/ChangeLog PR bootstrap/69123 * var-tracking.c (dump_onepart_variable_differences): New. (dataflow_set_different): If a detailed dump is requested, delay early returns and dump differences between onepart variables present before and after, and added variables. --- gcc/var-tracking.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 103 insertions(+), 10 deletions(-) diff --git a/gcc/var-tracking.c b/gcc/var-tracking.c index a5cca2b..634ebe0 100644 --- a/gcc/var-tracking.c +++ b/gcc/var-tracking.c @@ -4921,6 +4921,63 @@ onepart_variable_different_p (variable *var1, variable *var2) return lc1 != lc2; } +/* Return true if one-part variables VAR1 and VAR2 are different. + They must be in canonical order. */ + +static void +dump_onepart_variable_differences (variable *var1, variable *var2) +{ + location_chain *lc1, *lc2; + + gcc_assert (var1 != var2); + gcc_assert (dump_file); + gcc_assert (dv_as_opaque (var1->dv) == dv_as_opaque (var2->dv)); + gcc_assert (var1->n_var_parts == 1 + && var2->n_var_parts == 1); + + lc1 = var1->var_part[0].loc_chain; + lc2 = var2->var_part[0].loc_chain; + + gcc_assert (lc1 && lc2); + + while (lc1 && lc2) + { + switch (loc_cmp (lc1->loc, lc2->loc)) + { + case -1: + fprintf (dump_file, "removed: "); + print_rtl_single (dump_file, lc1->loc); + lc1 = lc1->next; + continue; + case 0: + break; + case 1: + fprintf (dump_file, "added: "); + print_rtl_single (dump_file, lc2->loc); + lc2 = lc2->next; + continue; + default: + gcc_unreachable (); + } + lc1 = lc1->next; + lc2 = lc2->next; + } + + while (lc1) + { + fprintf (dump_file, "removed: "); + print_rtl_single (dump_file, lc1->loc); + lc1 = lc1->next; + } + + while (lc2) + { + fprintf (dump_file, "added: "); + print_rtl_single (dump_file, lc2->loc); + lc2 = lc2->next; + } +} + /* Return true if variables VAR1 and VAR2 are different. */ static bool @@ -4964,19 +5021,32 @@ dataflow_set_different (dataflow_set *old_set, dataflow_set *new_set) { variable_iterator_type hi; variable *var1; + bool diffound = false; + bool details = (dump_file && (dump_flags & TDF_DETAILS)); + +#define RETRUE \ + do \ + { \ + if (!details) \ + return true; \ + else \ + diffound = true; \ + } \ + while (0) if (old_set->vars == new_set->vars) return false; if (shared_hash_htab (old_set->vars)->elements () != shared_hash_htab (new_set->vars)->elements ()) - return true; + RETRUE; FOR_EACH_HASH_TABLE_ELEMENT (*shared_hash_htab (old_set->vars), var1, variable, hi) { variable_table_type *htab = shared_hash_htab (new_set->vars); variable *var2 = htab->find_with_hash (var1->dv, dv_htab_hash (var1->dv)); + if (!var2) { if (dump_file && (dump_flags & TDF_DETAILS)) @@ -4984,26 +5054,49 @@ dataflow_set_different (dataflow_set *old_set, dataflow_set *new_set) fprintf (dump_file, "dataflow difference found: removal of:\n"); dump_var (var1); } - return true; + RETRUE; } - - if (variable_different_p (var1, var2)) + else if (variable_different_p (var1, var2)) { - if (dump_file && (dump_flags & TDF_DETAILS)) + if (details) { fprintf (dump_file, "dataflow difference found: " "old and new follow:\n"); dump_var (var1); + if (dv_onepart_p (var1->dv)) + dump_onepart_variable_differences (var1, var2); dump_var (var2); } - return true; + RETRUE; } } - /* No need to traverse the second hashtab, if both have the same number - of elements and the second one had all entries found in the first one, - then it can't have any extra entries. */ - return false; + /* There's no need to traverse the second hashtab unless we want to + print the details. If both have the same number of elements and + the second one had all entries found in the first one, then the + second can't have any extra entries. */ + if (!details) + return diffound; + + FOR_EACH_HASH_TABLE_ELEMENT (*shared_hash_htab (new_set->vars), + var1, variable, hi) + { + variable_table_type *htab = shared_hash_htab (old_set->vars); + variable *var2 = htab->find_with_hash (var1->dv, dv_htab_hash (var1->dv)); + if (!var2) + { + if (details) + { + fprintf (dump_file, "dataflow difference found: addition of:\n"); + dump_var (var1); + } + RETRUE; + } + } + +#undef RETRUE + + return diffound; } /* Free the contents of dataflow set SET. */