On 06/12/2018 10:44 PM, Jeff Law wrote: > On 06/05/2018 01:15 AM, marxin wrote: >> gcc/ChangeLog: >> >> 2018-06-07 Martin Liska <mli...@suse.cz> >> >> * tree-switch-conversion.c (switch_conversion::collect): >> Record m_uniq property. >> (switch_conversion::expand): Bail out for special conditions. >> (group_cluster::~group_cluster): New. >> (group_cluster::group_cluster): Likewise. >> (group_cluster::dump): Likewise. >> (jump_table_cluster::emit): New. >> (switch_decision_tree::fix_phi_operands_for_edges): New. >> (struct case_node): Remove struct. >> (jump_table_cluster::can_be_handled): New. >> (case_values_threshold): Moved to header. >> (reset_out_edges_aux): Likewise. >> (jump_table_cluster::is_beneficial): New. >> (bit_test_cluster::can_be_handled): Likewise. >> (add_case_node): Remove. >> (bit_test_cluster::is_beneficial): New. >> (case_bit_test::cmp): New. >> (bit_test_cluster::emit): New. >> (expand_switch_as_decision_tree_p): Remove. >> (bit_test_cluster::hoist_edge_and_branch_if_true): New. >> (fix_phi_operands_for_edge): Likewise. >> (switch_decision_tree::analyze_switch_statement): New. >> (compute_cases_per_edge): Move ... >> (switch_decision_tree::compute_cases_per_edge): ... here. >> (try_switch_expansion): Likewise. >> (switch_decision_tree::try_switch_expansion): Likewise. >> (record_phi_operand_mapping): Likewise. >> (switch_decision_tree::record_phi_operand_mapping): Likewise. >> (emit_case_decision_tree): Likewise. >> (switch_decision_tree::emit): Likewise. >> (balance_case_nodes): Likewise. >> (switch_decision_tree::balance_case_nodes): Likewise. >> (dump_case_nodes): Likewise. >> (switch_decision_tree::dump_case_nodes): Likewise. >> (emit_jump): Likewise. >> (switch_decision_tree::emit_jump): Likewise. >> (emit_cmp_and_jump_insns): Likewise. >> (switch_decision_tree::emit_cmp_and_jump_insns): Likewise. >> (emit_case_nodes): Likewise. >> (switch_decision_tree::emit_case_nodes): Likewise. >> (conditional_probability): Remove. >> * tree-switch-conversion.h (enum cluster_type): New. >> (PRINT_CASE): New. >> (struct cluster): Likewise. >> (cluster::cluster): Likewise. >> (struct simple_cluster): Likewise. >> (simple_cluster::simple_cluster): Likewise. >> (struct group_cluster): Likewise. >> (struct jump_table_cluster): Likewise. >> (struct bit_test_cluster): Likewise. >> (struct min_cluster_item): Likewise. >> (struct case_tree_node): Likewise. >> (case_tree_node::case_tree_node): Likewise. >> (jump_table_cluster::case_values_threshold): Likewise. >> (struct case_bit_test): Likewise. >> (struct switch_decision_tree): Likewise. >> (struct switch_conversion): Likewise. >> (switch_decision_tree::reset_out_edges_aux): Likewise. >> >> gcc/testsuite/ChangeLog: >> >> 2018-06-07 Martin Liska <mli...@suse.cz> >> >> * gcc.dg/tree-ssa/vrp104.c: Grep just for GIMPLE IL. > So like the previous patch, we need to make sure functions have their > function comments. > > >> --- >> gcc/testsuite/gcc.dg/tree-ssa/vrp104.c | 2 +- >> gcc/tree-switch-conversion.c | 1343 ++++++++++++++---------- >> gcc/tree-switch-conversion.h | 545 ++++++++++ >> 3 files changed, 1325 insertions(+), 565 deletions(-) >> >> >> 0002-Switch-other-switch-expansion-methods-into-classes.patch >> >> >> + The definition of "much bigger" depends on whether we are >> + optimizing for size or for speed. If the former, the maximum >> + ratio range/count = 3, because this was found to be the optimal >> + ratio for size on i686-pc-linux-gnu, see PR11823. The ratio >> + 10 is much older, and was probably selected after an extensive >> + benchmarking investigation on numerous platforms. Or maybe it >> + just made sense to someone at some point in the history of GCC, >> + who knows... */ > "much older" is an understatement. I believe the magic "10" pre-dates > my involvement in GCC. You can find evidence of it as far back as > gcc-0.9. I doubt it was extensively benchmarked, and even if it was, > the targets on which it was benchmarked don't reflect modern target > reality in terms of importance.
Good, then lets decrease it to 8 and we'll see if some regression will appear. > > In general this patch is much harder to de-cipher as there's little > relation between chunks of code in the unidiff. Given your long history > with GCC, I'm going to extend a lot of leeway that you got all this > stuff right as you moved code around. However, in the future for this > kind of change we should seriously look at a different way to break down > a patch into meaningful chunks. Thank you for the trust. I tried to split it into multiple patches, but it wasn't readable enough. Martin > > So, OK with adding the missing function comments. I think that covers > the entire set. > > Thanks, > Jeff >