From patchwork Tue Oct 24 18:44:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 830011 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-465031-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="hc4Azwjg"; dkim-atps=neutral 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 3yM2Hz2rp0z9sPr for ; Wed, 25 Oct 2017 05:44:47 +1100 (AEDT) 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:message-id:date:mime-version:content-type; q=dns; s= default; b=FZ/x0DYWYYZZpwFTpSWmJvOk/OQoqKSA+1kmMjl/CHX5DuVhZIYkA VnImW24W7GaNfjjvXCqi1NnaCJVB0wGeJcR8/f1CeeAe3JEhSZwxGJh/lCU8rXgB Q0bPl99TqmPzALbVWbHETyW5lYV+BebFzFTloULdOYW58xZbYjArHI= 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:message-id:date:mime-version:content-type; s= default; bh=hl7i73czEQxeb2sdaBocN3f1Nj8=; b=hc4AzwjgLrGT6oiZA1ZV D5c9rcoPNHyTZEInoiSj257mkYNqNUnEQBjS+MqzocMyzBG0DoWtRFWoT9RpGZN7 1DiqUXQVl7PtjwITScuAcWphq+bvpS1ELLfrhzT2+d63v40ndHuq7t3zaIY0F7/6 4f4yUrfe0xn3iJvj5JJ2Guk= Received: (qmail 46342 invoked by alias); 24 Oct 2017 18:44:38 -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 46296 invoked by uid 89); 24 Oct 2017 18:44:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= 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 ESMTP; Tue, 24 Oct 2017 18:44:36 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D71287CB95 for ; Tue, 24 Oct 2017 18:44:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D71287CB95 Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=law@redhat.com Received: from localhost.localdomain (ovpn-112-29.rdu2.redhat.com [10.10.112.29]) by smtp.corp.redhat.com (Postfix) with ESMTP id EA1C65C8B0 for ; Tue, 24 Oct 2017 18:44:33 +0000 (UTC) From: Jeff Law To: gcc-patches Subject: [RFA][PATCH] Provide a class interface into substitute_and_fold. Message-ID: Date: Tue, 24 Oct 2017 12:44:31 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 X-IsSubscribed: yes This is similar to the introduction of the ssa_propagate_engine, but for the substitution/replacements bits. In a couple places the pass specific virtual functions are just wrappers around existing functions. A good example of this is ccp_folder::get_value. Many other routines in tree-ssa-ccp.c want to use get_constant_value. Some may be convertable to use the class instance, but I haven't looked closely. Another example is vrp_folder::get_value. In this case we're wrapping op_with_constant_singleton_value. In a later patch that moves into the to-be-introduced vr_values class so we'll delegate to that class rather than wrap. FWIW I did look at having a single class for the propagation engine and the substitution engine. That turned out to be a bit problematical due to the calls into the substitution engine from the evrp bits which don't use the propagation engine at all. Given propagation and substitution are distinct concepts I ultimately decided the cleanest path forward was to keep the two classes separate. Bootstrapped and regression tested on x86_64. OK for the trunk? Jeff * tree-ssa-ccp.c (ccp_folder): New class derived from substitute_and_fold_engine. (ccp_folder::get_value): New member function. (ccp_folder::fold_stmt): Renamed from ccp_fold_stmt. (ccp_fold_stmt): Remove prototype. (ccp_finalize): Call substitute_and_fold from the ccp_class. * tree-ssa-copy.c (copy_folder): New class derived from substitute_and_fold_engine. (copy_folder::get_value): Renamed from get_value. (fini_copy_prop): Call substitute_and_fold from copy_folder class. * tree-vrp.c (vrp_folder): New class derived from substitute_and_fold_engine. (vrp_folder::fold_stmt): Renamed from vrp_fold_stmt. (vrp_folder::get_value): New member function. (vrp_finalize): Call substitute_and_fold from vrp_folder class. (evrp_dom_walker::before_dom_children): Similarly for replace_uses_in. * tree-ssa-propagate.h (substitute_and_fold_engine): New class to provide a class interface to folder/substitute routines. (ssa_prop_fold_stmt_fn): Remove typedef. (ssa_prop_get_value_fn): Likewise. (subsitute_and_fold): Remove prototype. (replace_uses_in): Likewise. * tree-ssa-propagate.c (substitute_and_fold_engine::replace_uses_in): Renamed from replace_uses_in. Call the virtual member function (substitute_and_fold_engine::replace_phi_args_in): Similarly. (substitute_and_fold_dom_walker): Remove initialization of data member entries for calbacks. Add substitute_and_fold_engine member and initialize it. (substitute_and_fold_dom_walker::before_dom_children0: Use the member functions for get_value, replace_phi_args_in c replace_uses_in, and fold_stmt calls. (substitute_and_fold_engine::substitute_and_fold): Renamed from substitute_and_fold. Remove assert. Update ctor call. diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c index fec562e..da06172 100644 --- a/gcc/tree-ssa-ccp.c +++ b/gcc/tree-ssa-ccp.c @@ -188,7 +188,6 @@ static ccp_prop_value_t *const_val; static unsigned n_const_val; static void canonicalize_value (ccp_prop_value_t *); -static bool ccp_fold_stmt (gimple_stmt_iterator *); static void ccp_lattice_meet (ccp_prop_value_t *, ccp_prop_value_t *); /* Dump constant propagation value VAL to file OUTF prefixed by PREFIX. */ @@ -909,6 +908,24 @@ do_dbg_cnt (void) } +/* We want to provide our own GET_VALUE and FOLD_STMT virtual methods. */ +class ccp_folder : public substitute_and_fold_engine +{ + public: + tree get_value (tree); + bool fold_stmt (gimple_stmt_iterator *); +}; + +/* This method just wraps GET_CONSTANT_VALUE for now. Over time + naked calls to GET_CONSTANT_VALUE should be eliminated in favor + of calling member functions. */ + +tree +ccp_folder::get_value (tree op) +{ + return get_constant_value (op); +} + /* Do final substitution of propagated values, cleanup the flowgraph and free allocated storage. If NONZERO_P, record nonzero bits. @@ -967,7 +984,8 @@ ccp_finalize (bool nonzero_p) } /* Perform substitutions based on the known constant values. */ - something_changed = substitute_and_fold (get_constant_value, ccp_fold_stmt); + class ccp_folder ccp_folder; + something_changed = ccp_folder.substitute_and_fold (); free (const_val); const_val = NULL; @@ -2176,8 +2194,8 @@ fold_builtin_alloca_with_align (gimple *stmt) /* Fold the stmt at *GSI with CCP specific information that propagating and regular folding does not catch. */ -static bool -ccp_fold_stmt (gimple_stmt_iterator *gsi) +bool +ccp_folder::fold_stmt (gimple_stmt_iterator *gsi) { gimple *stmt = gsi_stmt (*gsi); diff --git a/gcc/tree-ssa-copy.c b/gcc/tree-ssa-copy.c index a57535c..e45d188 100644 --- a/gcc/tree-ssa-copy.c +++ b/gcc/tree-ssa-copy.c @@ -489,10 +489,16 @@ init_copy_prop (void) } } +class copy_folder : public substitute_and_fold_engine +{ + public: + tree get_value (tree); +}; + /* Callback for substitute_and_fold to get at the final copy-of values. */ -static tree -get_value (tree name) +tree +copy_folder::get_value (tree name) { tree val; if (SSA_NAME_VERSION (name) >= n_copy_of) @@ -557,7 +563,8 @@ fini_copy_prop (void) } } - bool changed = substitute_and_fold (get_value, NULL); + class copy_folder copy_folder; + bool changed = copy_folder.substitute_and_fold (); if (changed) { free_numbers_of_iterations_estimates (cfun); diff --git a/gcc/tree-ssa-propagate.c b/gcc/tree-ssa-propagate.c index 90df285..62955be 100644 --- a/gcc/tree-ssa-propagate.c +++ b/gcc/tree-ssa-propagate.c @@ -853,7 +853,7 @@ static struct prop_stats_d prop_stats; PROP_VALUE. Return true if at least one reference was replaced. */ bool -replace_uses_in (gimple *stmt, ssa_prop_get_value_fn get_value) +substitute_and_fold_engine::replace_uses_in (gimple *stmt) { bool replaced = false; use_operand_p use; @@ -862,7 +862,7 @@ replace_uses_in (gimple *stmt, ssa_prop_get_value_fn get_value) FOR_EACH_SSA_USE_OPERAND (use, stmt, iter, SSA_OP_USE) { tree tuse = USE_FROM_PTR (use); - tree val = (*get_value) (tuse); + tree val = get_value (tuse); if (val == tuse || val == NULL_TREE) continue; @@ -891,8 +891,8 @@ replace_uses_in (gimple *stmt, ssa_prop_get_value_fn get_value) /* Replace propagated values into all the arguments for PHI using the values from PROP_VALUE. */ -static bool -replace_phi_args_in (gphi *phi, ssa_prop_get_value_fn get_value) +bool +substitute_and_fold_engine::replace_phi_args_in (gphi *phi) { size_t i; bool replaced = false; @@ -909,7 +909,7 @@ replace_phi_args_in (gphi *phi, ssa_prop_get_value_fn get_value) if (TREE_CODE (arg) == SSA_NAME) { - tree val = (*get_value) (arg); + tree val = get_value (arg); if (val && val != arg && may_propagate_copy (arg, val)) { @@ -960,10 +960,10 @@ class substitute_and_fold_dom_walker : public dom_walker { public: substitute_and_fold_dom_walker (cdi_direction direction, - ssa_prop_get_value_fn get_value_fn_, - ssa_prop_fold_stmt_fn fold_fn_) - : dom_walker (direction), get_value_fn (get_value_fn_), - fold_fn (fold_fn_), something_changed (false) + class substitute_and_fold_engine *engine) + : dom_walker (direction), + something_changed (false), + substitute_and_fold_engine (engine) { stmts_to_remove.create (0); stmts_to_fixup.create (0); @@ -979,12 +979,12 @@ public: virtual edge before_dom_children (basic_block); virtual void after_dom_children (basic_block) {} - ssa_prop_get_value_fn get_value_fn; - ssa_prop_fold_stmt_fn fold_fn; bool something_changed; vec stmts_to_remove; vec stmts_to_fixup; bitmap need_eh_cleanup; + + class substitute_and_fold_engine *substitute_and_fold_engine; }; edge @@ -1001,7 +1001,7 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb) continue; if (res && TREE_CODE (res) == SSA_NAME) { - tree sprime = get_value_fn (res); + tree sprime = substitute_and_fold_engine->get_value (res); if (sprime && sprime != res && may_propagate_copy (res, sprime)) @@ -1010,7 +1010,7 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb) continue; } } - something_changed |= replace_phi_args_in (phi, get_value_fn); + something_changed |= substitute_and_fold_engine->replace_phi_args_in (phi); } /* Propagate known values into stmts. In some case it exposes @@ -1027,7 +1027,7 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb) tree lhs = gimple_get_lhs (stmt); if (lhs && TREE_CODE (lhs) == SSA_NAME) { - tree sprime = get_value_fn (lhs); + tree sprime = substitute_and_fold_engine->get_value (lhs); if (sprime && sprime != lhs && may_propagate_copy (lhs, sprime) @@ -1056,7 +1056,7 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb) && gimple_call_noreturn_p (stmt)); /* Replace real uses in the statement. */ - did_replace |= replace_uses_in (stmt, get_value_fn); + did_replace |= substitute_and_fold_engine->replace_uses_in (stmt); /* If we made a replacement, fold the statement. */ if (did_replace) @@ -1069,16 +1069,13 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb) /* Some statements may be simplified using propagator specific information. Do this before propagating into the stmt to not disturb pass specific information. */ - if (fold_fn) + update_stmt_if_modified (stmt); + if (substitute_and_fold_engine->fold_stmt(&i)) { - update_stmt_if_modified (stmt); - if ((*fold_fn)(&i)) - { - did_replace = true; - prop_stats.num_stmts_folded++; - stmt = gsi_stmt (i); - gimple_set_modified (stmt, true); - } + did_replace = true; + prop_stats.num_stmts_folded++; + stmt = gsi_stmt (i); + gimple_set_modified (stmt, true); } /* If this is a control statement the propagator left edges @@ -1164,19 +1161,15 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb) Return TRUE when something changed. */ bool -substitute_and_fold (ssa_prop_get_value_fn get_value_fn, - ssa_prop_fold_stmt_fn fold_fn) +substitute_and_fold_engine::substitute_and_fold (void) { - gcc_assert (get_value_fn); - if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, "\nSubstituting values and folding statements\n\n"); memset (&prop_stats, 0, sizeof (prop_stats)); calculate_dominance_info (CDI_DOMINATORS); - substitute_and_fold_dom_walker walker(CDI_DOMINATORS, - get_value_fn, fold_fn); + substitute_and_fold_dom_walker walker (CDI_DOMINATORS, this); walker.walk (ENTRY_BLOCK_PTR_FOR_FN (cfun)); /* We cannot remove stmts during the BB walk, especially not release diff --git a/gcc/tree-ssa-propagate.h b/gcc/tree-ssa-propagate.h index cff9e53..629ae77 100644 --- a/gcc/tree-ssa-propagate.h +++ b/gcc/tree-ssa-propagate.h @@ -61,17 +61,11 @@ enum ssa_prop_result { }; -/* Call-back functions used by the value propagation engine. */ -typedef bool (*ssa_prop_fold_stmt_fn) (gimple_stmt_iterator *gsi); -typedef tree (*ssa_prop_get_value_fn) (tree); - - extern bool valid_gimple_rhs_p (tree); extern void move_ssa_defining_stmt_for_defs (gimple *, gimple *); extern bool update_gimple_call (gimple_stmt_iterator *, tree, int, ...); extern bool update_call_from_tree (gimple_stmt_iterator *, tree); extern bool stmt_makes_single_store (gimple *); -extern bool substitute_and_fold (ssa_prop_get_value_fn, ssa_prop_fold_stmt_fn); extern bool may_propagate_copy (tree, tree); extern bool may_propagate_copy_into_stmt (gimple *, tree); extern bool may_propagate_copy_into_asm (tree); @@ -79,7 +73,6 @@ extern void propagate_value (use_operand_p, tree); extern void replace_exp (use_operand_p, tree); extern void propagate_tree_value (tree *, tree); extern void propagate_tree_value_into_stmt (gimple_stmt_iterator *, tree); -extern bool replace_uses_in (gimple *stmt, ssa_prop_get_value_fn get_value); /* Public interface into the SSA propagation engine. Clients should inherit from this class and provide their own visitors. */ @@ -104,4 +97,14 @@ class ssa_propagation_engine }; +class substitute_and_fold_engine +{ + public: + bool substitute_and_fold (void); + bool replace_uses_in (gimple *); + virtual bool fold_stmt (gimple_stmt_iterator *) { return false; } + virtual tree get_value (tree) { return NULL_TREE; } + bool replace_phi_args_in (gphi *); +}; + #endif /* _TREE_SSA_PROPAGATE_H */ diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index e8fce3e..ea8ceee 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -10536,10 +10536,17 @@ fold_predicate_in (gimple_stmt_iterator *si) return false; } +class vrp_folder : public substitute_and_fold_engine +{ + public: + tree get_value (tree); + bool fold_stmt (gimple_stmt_iterator *); +}; + /* Callback for substitute_and_fold folding the stmt at *SI. */ -static bool -vrp_fold_stmt (gimple_stmt_iterator *si) +bool +vrp_folder::fold_stmt (gimple_stmt_iterator *si) { if (fold_predicate_in (si)) return true; @@ -10547,6 +10554,18 @@ vrp_fold_stmt (gimple_stmt_iterator *si) return simplify_stmt_using_ranges (si); } +/* If OP has a value range with a single constant value return that, + otherwise return NULL_TREE. This returns OP itself if OP is a + constant. + + Implemented as a pure wrapper right now, but this will change. */ + +tree +vrp_folder::get_value (tree op) +{ + return op_with_constant_singleton_value_range (op); +} + /* Return the LHS of any ASSERT_EXPR where OP appears as the first argument to the ASSERT_EXPR and in which the ASSERT_EXPR dominates BB. If no such ASSERT_EXPR is found, return OP. */ @@ -10888,7 +10907,8 @@ vrp_finalize (bool warn_array_bounds_p) wi::to_wide (vr_value[i]->max)); } - substitute_and_fold (op_with_constant_singleton_value_range, vrp_fold_stmt); + class vrp_folder vrp_folder; + vrp_folder.substitute_and_fold (); if (warn_array_bounds && warn_array_bounds_p) check_all_array_refs (); @@ -11225,8 +11245,8 @@ evrp_dom_walker::before_dom_children (basic_block bb) } /* Try folding stmts with the VR discovered. */ - bool did_replace - = replace_uses_in (stmt, op_with_constant_singleton_value_range); + class vrp_folder vrp_folder; + bool did_replace = vrp_folder.replace_uses_in (stmt); if (fold_stmt (&gsi, follow_single_use_edges) || did_replace) {