From patchwork Tue Dec 1 18:33:59 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Lawrence X-Patchwork-Id: 550994 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 968DC140291 for ; Wed, 2 Dec 2015 05:34:27 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=H9Cjkkir; 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:cc:subject:date:message-id:content-type :content-transfer-encoding; q=dns; s=default; b=bolQoxnjVnMglQ8R hsgvxpFm9zPbcm9BU/IDNp2zpLd7piv6UKcvfx81Svg+FMcOXCpljtMAVtM+pO7N 5a7bof2b8ACWpze7ScVn5POaqSHRixpxo+Hf6qA1I7NXWu/YUrpnaKGAC9aEN1+H teU8DQuzmGEJM6e9zt245My1kYo= 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:cc:subject:date:message-id:content-type :content-transfer-encoding; s=default; bh=PDtvi4+rEUVmZI5t/GT4k3 7dypM=; b=H9Cjkkir9NuGurvehffY916Q58TiTmd1cYP8oPd5dKMDEO859fc4OM Gv2uLYeGM5bKBzoUt3dOnmI5wKBYywAjDYF8UtIEv4r1HTOJm18Xl2NS99s6ZOb4 daGDkm6pCcj7WOvnf2iYXpYGcw1L231lkMCghBu+paS/8CwokB+0s= Received: (qmail 7818 invoked by alias); 1 Dec 2015 18:34:20 -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 7804 invoked by uid 89); 1 Dec 2015 18:34:19 -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_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 01 Dec 2015 18:34:17 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-6-fTvS_l6VR4eduL_2gmf7Ew-1; Tue, 01 Dec 2015 18:34:11 +0000 Received: from arm.com ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 1 Dec 2015 18:34:11 +0000 From: Alan Lawrence To: gcc-patches@gcc.gnu.org Cc: rguenther@suse.de, law@redhat.com, jakub@redhat.com Subject: [PATCH] Empty redirect_edge_var_map after each pass and function Date: Tue, 1 Dec 2015 18:33:59 +0000 Message-Id: <1448994839-10665-1-git-send-email-alan.lawrence@arm.com> X-MC-Unique: fTvS_l6VR4eduL_2gmf7Ew-1 X-IsSubscribed: yes This follows on from discussion at https://gcc.gnu.org/ml/gcc-patches/2015-11/msg03392.html To recap: Starting in r229479 and continuing until at least 229711, compiling polynom.c from spec2000 on aarch64-none-linux-gnu, with options -O3 -mcpu=cortex-a53 -ffast-math (on both cross, native bootstrapped, and native --disable-bootstrap compilers), produced a verify_gimple ICE after unswitch: ../spec2000/benchspec/CINT2000/254.gap/src/polynom.c: In function 'NormalizeCoeffsListx': ../spec2000/benchspec/CINT2000/254.gap/src/polynom.c:358:11: error: incompatible types in PHI argument 0 TypHandle NormalizeCoeffsListx ( hdC ) ^ long int int ../spec2000/benchspec/CINT2000/254.gap/src/polynom.c:358:11: error: location references block not in block tree l1_279 = PHI <1(28), l1_299(33)> ../spec2000/benchspec/CINT2000/254.gap/src/polynom.c:358:11: error: invalid PHI argument ../spec2000/benchspec/CINT2000/254.gap/src/polynom.c:358:11: internal compiler error: tree check: expected class 'type', have 'declaration' (namespace_decl) in useless_type_conversion_p, at gimple-expr.c:84 0xd110ef tree_class_check_failed(tree_node const*, tree_code_class, char const*, int, char const*) ../../gcc-fsf/gcc/tree.c:9643 0x82561b tree_class_check ../../gcc-fsf/gcc/tree.h:3042 0x82561b useless_type_conversion_p(tree_node*, tree_node*) ../../gcc-fsf/gcc/gimple-expr.c:84 0xaca043 verify_gimple_phi ../../gcc-fsf/gcc/tree-cfg.c:4673 0xaca043 verify_gimple_in_cfg(function*, bool) ../../gcc-fsf/gcc/tree-cfg.c:4967 0x9c2e0b execute_function_todo ../../gcc-fsf/gcc/passes.c:1967 0x9c360b do_per_function ../../gcc-fsf/gcc/passes.c:1659 0x9c3807 execute_todo ../../gcc-fsf/gcc/passes.c:2022 I was not able to reduce the testcase below about 30k characters, with e.g. #define T_VOID 0 .... T_VOID .... producing the ICE, but manually changing to .... 0 .... preventing the ICE; as did running the preprocessor as a separate step, or a wide variety of options (e.g. -fdump-tree-alias). In the end I traced this to loop_unswitch reading stale values from the edge redirect map, which is keyed on 'edge' (a pointer to struct edge_def); the map entries had been left there by pass_dominator (on a different function), and by "chance" the edge *pointers* were the same as to some current edge_defs (even though they pointed to structures created by different allocations, the first of which had since been freed). Hence the fragility of the testcase and environment. While the ICE is prevented merely by adding a call to redirect_edge_var_map_destroy at the end of pass_dominator::execute, given the fragility of the bug, difficulty of reducing the testcase, and the low overhead of emptying an already-empty map, I believe the right fix is to empty the map as often as can correctly do so, hence this patch - based substantially on Richard's comments in PR/68117. Bootstrapped + check-gcc + check-g++ on x86_64 linux, based on r231105; I've also built SPEC2000 on aarch64-none-linux-gnu by applying this patch (roughly) onto the previously-failing r229711, which also passes aarch64 bootstrap, and a more recent bootstrap on aarch64 is ongoing. Assuming/if no regressions there... Is this ok for trunk? This could also be a candidate for the 5.3 release; backporting depends only on the (fairly trivial) r230357. gcc/ChangeLog: Alan Lawrence Richard Biener * cfgexpand.c (pass_expand::execute): Replace call to redirect_edge_var_map_destroy with redirect_edge_var_map_empty. * tree-ssa.c (delete_tree_ssa): Likewise. * function.c (set_cfun): Call redirect_edge_var_map_empty. * passes.c (execute_one_ipa_transform_pass, execute_one_pass): Likewise. * tree-ssa.h (redirect_edge_var_map_destroy): Remove. (redirect_edge_var_map_empty): New. * tree-ssa.c (redirect_edge_var_map_destroy): Remove. (redirect_edge_var_map_empty): New. --- gcc/cfgexpand.c | 2 +- gcc/function.c | 2 ++ gcc/passes.c | 2 ++ gcc/tree-ssa.c | 8 ++++---- gcc/tree-ssa.h | 2 +- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index 1990e10..ede1b82 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -6291,7 +6291,7 @@ pass_expand::execute (function *fun) expand_phi_nodes (&SA); /* Release any stale SSA redirection data. */ - redirect_edge_var_map_destroy (); + redirect_edge_var_map_empty (); /* Register rtl specific functions for cfg. */ rtl_register_cfg_hooks (); diff --git a/gcc/function.c b/gcc/function.c index 515d7c0..e452865 100644 --- a/gcc/function.c +++ b/gcc/function.c @@ -75,6 +75,7 @@ along with GCC; see the file COPYING3. If not see #include "tree-chkp.h" #include "rtl-chkp.h" #include "tree-dfa.h" +#include "tree-ssa.h" /* So we can assign to cfun in this file. */ #undef cfun @@ -4798,6 +4799,7 @@ set_cfun (struct function *new_cfun) { cfun = new_cfun; invoke_set_current_function_hook (new_cfun ? new_cfun->decl : NULL_TREE); + redirect_edge_var_map_empty (); } } diff --git a/gcc/passes.c b/gcc/passes.c index 0e23dcb..ba9bfc2 100644 --- a/gcc/passes.c +++ b/gcc/passes.c @@ -2218,6 +2218,7 @@ execute_one_ipa_transform_pass (struct cgraph_node *node, pass_fini_dump_file (pass); current_pass = NULL; + redirect_edge_var_map_empty (); /* Signal this is a suitable GC collection point. */ if (!(todo_after & TODO_do_not_ggc_collect)) @@ -2370,6 +2371,7 @@ execute_one_pass (opt_pass *pass) || pass->type != RTL_PASS); current_pass = NULL; + redirect_edge_var_map_empty (); if (todo_after & TODO_discard_function) { diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c index 02fca4c..ddc7a65 100644 --- a/gcc/tree-ssa.c +++ b/gcc/tree-ssa.c @@ -119,10 +119,10 @@ redirect_edge_var_map_vector (edge e) /* Clear the edge variable mappings. */ void -redirect_edge_var_map_destroy (void) +redirect_edge_var_map_empty (void) { - delete edge_var_maps; - edge_var_maps = NULL; + if (edge_var_maps) + edge_var_maps->empty (); } @@ -1128,7 +1128,7 @@ delete_tree_ssa (struct function *fn) fn->gimple_df = NULL; /* We no longer need the edge variable maps. */ - redirect_edge_var_map_destroy (); + redirect_edge_var_map_empty (); } /* Return true if EXPR is a useless type conversion, otherwise return diff --git a/gcc/tree-ssa.h b/gcc/tree-ssa.h index 3b5bd70..d33eb9c 100644 --- a/gcc/tree-ssa.h +++ b/gcc/tree-ssa.h @@ -35,7 +35,7 @@ extern void redirect_edge_var_map_add (edge, tree, tree, source_location); extern void redirect_edge_var_map_clear (edge); extern void redirect_edge_var_map_dup (edge, edge); extern vec *redirect_edge_var_map_vector (edge); -extern void redirect_edge_var_map_destroy (void); +extern void redirect_edge_var_map_empty (void); extern edge ssa_redirect_edge (edge, basic_block); extern void flush_pending_stmts (edge); extern void gimple_replace_ssa_lhs (gimple *, tree);