From patchwork Thu Nov 12 11:37:45 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 543316 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 0AABC1401E7 for ; Thu, 12 Nov 2015 22:38:34 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=bgLvIc+I; 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 :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=IIgnRpVMfqBJIhNnE/+5h6m9Sb8lXwIdwgu8UX2o3h0A15uSaj dtI+Y3iXSnKIvoLR2tC/k1SwaUxMu0T5sCcMgmjSYNWfWsf0Vz98vssxxjegp9GQ p0XRPmuuexGknxDcCc5mHnXC5+TJsFib5kNU/fs/QQKUcuaUpiuIdBzHc= 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 :from:subject:message-id:date:mime-version:content-type; s= default; bh=I9HZxHYSz6fC+tkVk7j8xd8LRSo=; b=bgLvIc+INNde7Qq7JiIR 6+rhoNEam3xYMz1TDGxc2kiqPuAseGQX8n+wnOa6ibM3eNMkZAA2Lw6nO3PAiT/n 0OMyoz1xO6O4XRJsbCrFH2epD/O0d322TH2fLTDoSwbJ6JBfIvwRrbd4mK5CjcMu pisXQiJwkv0Vt0R0yDYvHps= Received: (qmail 66040 invoked by alias); 12 Nov 2015 11:38:28 -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 66019 invoked by uid 89); 12 Nov 2015 11:38:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: fencepost.gnu.org Received: from fencepost.gnu.org (HELO fencepost.gnu.org) (208.118.235.10) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 12 Nov 2015 11:38:24 +0000 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38269) by fencepost.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1ZwqCr-0008Jn-W4 for gcc-patches@gnu.org; Thu, 12 Nov 2015 06:38:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZwqCo-0005t3-7p for gcc-patches@gnu.org; Thu, 12 Nov 2015 06:38:21 -0500 Received: from relay1.mentorg.com ([192.94.38.131]:38559) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwqCn-0005sM-V1 for gcc-patches@gnu.org; Thu, 12 Nov 2015 06:38:18 -0500 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-FEM-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1ZwqCk-0007I4-IF from Tom_deVries@mentor.com for gcc-patches@gnu.org; Thu, 12 Nov 2015 03:38:15 -0800 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.3.224.2; Thu, 12 Nov 2015 11:38:12 +0000 To: "gcc-patches@gnu.org" From: Tom de Vries Subject: [RFC] Remove first_pass_instance from pass_vrp Message-ID: <56447A09.4070608@mentor.com> Date: Thu, 12 Nov 2015 12:37:45 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Windows NT kernel [generic] [fuzzy] X-Received-From: 192.94.38.131 Hi, [ See also related discussion at https://gcc.gnu.org/ml/gcc-patches/2012-07/msg00452.html ] this patch removes the usage of first_pass_instance from pass_vrp. the patch: - limits itself to pass_vrp, but my intention is to remove all usage of first_pass_instance - lacks an update to gdbhooks.py Modifying the pass behaviour depending on the instance number, as first_pass_instance does, break compositionality of the pass list. In other words, adding a pass instance in a pass list may change the behaviour of another instance of that pass in the pass list. Which obviously makes it harder to understand and change the pass list. [ I've filed this issue as PR68247 - Remove pass_first_instance ] The solution is to make the difference in behaviour explicit in the pass list, and no longer change behaviour depending on instance number. One obvious possible fix is to create a duplicate pass with a different name, say 'pass_vrp_warn_array_bounds': ... NEXT_PASS (pass_vrp_warn_array_bounds); ... NEXT_PASS (pass_vrp); ... But, AFAIU that requires us to choose a different dump-file name for each pass. And choosing vrp1 and vrp2 as new dump-file names still means that -fdump-tree-vrp no longer works (which was mentioned as drawback here: https://gcc.gnu.org/ml/gcc-patches/2012-07/msg00453.html ). This patch instead makes pass creation parameterizable. So in the pass list, we use: ... NEXT_PASS_WITH_ARG (pass_vrp, true /* warn_array_bounds_p */); ... NEXT_PASS_WITH_ARG (pass_vrp, false /* warn_array_bounds_p */); ... This approach gives us clarity in the pass list, similar to using a duplicate pass 'pass_vrp_warn_array_bounds'. But it also means -fdump-tree-vrp still works as before. Good idea? Other comments? Thanks, - Tom Remove first_pass_instance from pass_vrp --- gcc/gen-pass-instances.awk | 32 ++++++++++++++++++++++---------- gcc/pass_manager.h | 2 ++ gcc/passes.c | 20 ++++++++++++++++++++ gcc/passes.def | 4 ++-- gcc/tree-pass.h | 3 ++- gcc/tree-vrp.c | 22 ++++++++++++---------- 6 files changed, 60 insertions(+), 23 deletions(-) diff --git a/gcc/gen-pass-instances.awk b/gcc/gen-pass-instances.awk index cbfaa86..c77bd64 100644 --- a/gcc/gen-pass-instances.awk +++ b/gcc/gen-pass-instances.awk @@ -43,7 +43,7 @@ function handle_line() line = $0; # Find call expression. - call_starts_at = match(line, /NEXT_PASS \(.+\)/); + call_starts_at = match(line, /NEXT_PASS(_WITH_ARG)? \(.+\)/); if (call_starts_at == 0) { print line; @@ -53,23 +53,28 @@ function handle_line() # Length of the call expression. len_of_call = RLENGTH; - len_of_start = length("NEXT_PASS ("); len_of_open = length("("); len_of_close = length(")"); - # Find pass_name argument - len_of_pass_name = len_of_call - (len_of_start + len_of_close); - pass_starts_at = call_starts_at + len_of_start; - pass_name = substr(line, pass_starts_at, len_of_pass_name); - # Find call expression prefix (until and including called function) - prefix_len = pass_starts_at - 1 - len_of_open; - prefix = substr(line, 1, prefix_len); + match(line, /NEXT_PASS(_WITH_ARG)? /) + len_of_call_name = RLENGTH + prefix_len = call_starts_at + len_of_call_name - 1 + prefix = substr(line, 1, prefix_len) # Find call expression postfix postfix_starts_at = call_starts_at + len_of_call; postfix = substr(line, postfix_starts_at); + args_starts_at = prefix_len + 1 + len_of_open; + len_of_args = postfix_starts_at - args_starts_at - len_of_close; + args_str = substr(line, args_starts_at, len_of_args); + split(args_str, args, ","); + + # Find pass_name argument, an optional with_arg argument + pass_name = args[1]; + with_arg = args[2]; + # Set pass_counts if (pass_name in pass_counts) pass_counts[pass_name]++; @@ -79,7 +84,14 @@ function handle_line() pass_num = pass_counts[pass_name]; # Print call expression with extra pass_num argument - printf "%s(%s, %s)%s\n", prefix, pass_name, pass_num, postfix; + printf "%s(", prefix; + printf "%s", pass_name; + printf ", %s", pass_num; + if (with_arg) + { + printf ", %s", with_arg; + } + printf ")%s\n", postfix; } { handle_line() } diff --git a/gcc/pass_manager.h b/gcc/pass_manager.h index 7d539e4..a8199e2 100644 --- a/gcc/pass_manager.h +++ b/gcc/pass_manager.h @@ -120,6 +120,7 @@ private: #define PUSH_INSERT_PASSES_WITHIN(PASS) #define POP_INSERT_PASSES() #define NEXT_PASS(PASS, NUM) opt_pass *PASS ## _ ## NUM +#define NEXT_PASS_WITH_ARG(PASS, NUM, ARG) NEXT_PASS (PASS, NUM) #define TERMINATE_PASS_LIST() #include "pass-instances.def" @@ -128,6 +129,7 @@ private: #undef PUSH_INSERT_PASSES_WITHIN #undef POP_INSERT_PASSES #undef NEXT_PASS +#undef NEXT_PASS_WITH_ARG #undef TERMINATE_PASS_LIST }; // class pass_manager diff --git a/gcc/passes.c b/gcc/passes.c index dd8d00a..0fd365e 100644 --- a/gcc/passes.c +++ b/gcc/passes.c @@ -81,6 +81,12 @@ opt_pass::clone () internal_error ("pass %s does not support cloning", name); } +opt_pass * +opt_pass::clone_with_arg (bool) +{ + internal_error ("pass %s does not support cloning", name); +} + bool opt_pass::gate (function *) { @@ -1572,6 +1578,19 @@ pass_manager::pass_manager (context *ctxt) p = next_pass_1 (p, PASS ## _ ## NUM, PASS ## _1); \ } while (0) +#define NEXT_PASS_WITH_ARG(PASS, NUM, ARG) \ + do { \ + gcc_assert (NULL == PASS ## _ ## NUM); \ + if ((NUM) == 1) \ + PASS ## _1 = make_##PASS (m_ctxt, ARG); \ + else \ + { \ + gcc_assert (PASS ## _1); \ + PASS ## _ ## NUM = PASS ## _1->clone_with_arg (ARG); \ + } \ + p = next_pass_1 (p, PASS ## _ ## NUM, PASS ## _1); \ + } while (0) + #define TERMINATE_PASS_LIST() \ *p = NULL; @@ -1581,6 +1600,7 @@ pass_manager::pass_manager (context *ctxt) #undef PUSH_INSERT_PASSES_WITHIN #undef POP_INSERT_PASSES #undef NEXT_PASS +#undef NEXT_PASS_WITH_ARG #undef TERMINATE_PASS_LIST /* Register the passes with the tree dump code. */ diff --git a/gcc/passes.def b/gcc/passes.def index c0ab6b9..2649d67 100644 --- a/gcc/passes.def +++ b/gcc/passes.def @@ -171,7 +171,7 @@ along with GCC; see the file COPYING3. If not see NEXT_PASS (pass_return_slot); NEXT_PASS (pass_fre); NEXT_PASS (pass_merge_phi); - NEXT_PASS (pass_vrp); + NEXT_PASS_WITH_ARG (pass_vrp, true /* warn_array_bounds_p */); NEXT_PASS (pass_chkp_opt); NEXT_PASS (pass_dce); NEXT_PASS (pass_stdarg); @@ -280,7 +280,7 @@ along with GCC; see the file COPYING3. If not see NEXT_PASS (pass_tracer); NEXT_PASS (pass_dominator); NEXT_PASS (pass_strlen); - NEXT_PASS (pass_vrp); + NEXT_PASS_WITH_ARG (pass_vrp, false /* warn_array_bounds_p */); /* The only const/copy propagation opportunities left after DOM and VRP should be due to degenerate PHI nodes. So rather than run the full propagators, run a specialized pass which diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h index 49e22a9..0e330dd 100644 --- a/gcc/tree-pass.h +++ b/gcc/tree-pass.h @@ -83,6 +83,7 @@ public: The default implementation prints an error message and aborts. */ virtual opt_pass *clone (); + virtual opt_pass *clone_with_arg (bool); /* This pass and all sub-passes are executed only if the function returns true. The default implementation returns true. */ @@ -439,7 +440,7 @@ extern gimple_opt_pass *make_pass_fre (gcc::context *ctxt); extern gimple_opt_pass *make_pass_check_data_deps (gcc::context *ctxt); extern gimple_opt_pass *make_pass_copy_prop (gcc::context *ctxt); extern gimple_opt_pass *make_pass_isolate_erroneous_paths (gcc::context *ctxt); -extern gimple_opt_pass *make_pass_vrp (gcc::context *ctxt); +extern gimple_opt_pass *make_pass_vrp (gcc::context *ctxt, bool); extern gimple_opt_pass *make_pass_uncprop (gcc::context *ctxt); extern gimple_opt_pass *make_pass_return_slot (gcc::context *ctxt); extern gimple_opt_pass *make_pass_reassoc (gcc::context *ctxt); diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index e2393e4..0ff60fd 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -10183,7 +10183,7 @@ finalize_jump_threads (void) /* Traverse all the blocks folding conditionals with known ranges. */ static void -vrp_finalize (void) +vrp_finalize (bool warn_array_bounds_p) { size_t i; @@ -10199,7 +10199,7 @@ vrp_finalize (void) substitute_and_fold (op_with_constant_singleton_value_range, vrp_fold_stmt, false); - if (warn_array_bounds && first_pass_instance) + if (warn_array_bounds && warn_array_bounds_p) check_all_array_refs (); /* We must identify jump threading opportunities before we release @@ -10289,7 +10289,7 @@ vrp_finalize (void) probabilities to aid branch prediction. */ static unsigned int -execute_vrp (void) +execute_vrp (bool warn_array_bounds_p) { int i; edge e; @@ -10313,7 +10313,7 @@ execute_vrp (void) vrp_initialize (); ssa_propagate (vrp_visit_stmt, vrp_visit_phi_node); - vrp_finalize (); + vrp_finalize (warn_array_bounds_p); free_numbers_of_iterations_estimates (cfun); @@ -10385,21 +10385,23 @@ const pass_data pass_data_vrp = class pass_vrp : public gimple_opt_pass { public: - pass_vrp (gcc::context *ctxt) - : gimple_opt_pass (pass_data_vrp, ctxt) + pass_vrp (gcc::context *ctxt, bool warn_array_bounds_p) + : gimple_opt_pass (pass_data_vrp, ctxt), warn_array_bounds_p(warn_array_bounds_p) {} /* opt_pass methods: */ - opt_pass * clone () { return new pass_vrp (m_ctxt); } + opt_pass * clone (bool warn_array_bounds_p) { return new pass_vrp (m_ctxt, warn_array_bounds_p); } virtual bool gate (function *) { return flag_tree_vrp != 0; } - virtual unsigned int execute (function *) { return execute_vrp (); } + virtual unsigned int execute (function *) { return execute_vrp (warn_array_bounds_p); } + private: + bool warn_array_bounds_p; }; // class pass_vrp } // anon namespace gimple_opt_pass * -make_pass_vrp (gcc::context *ctxt) +make_pass_vrp (gcc::context *ctxt, bool warn_array_bounds_p) { - return new pass_vrp (ctxt); + return new pass_vrp (ctxt, warn_array_bounds_p); }