On 06/20/2018 02:47 AM, Martin Liška wrote: > 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. Agreed.
>> >> 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. Yea, it can be painful to find the right way to structure a series. In fact, I'm going to be faced with that shortly for a target change my son and I have in the works (converting v850 away from cc0). In the end it looks like 75% of the file changed, but a lot of it is just unidiff presenting the changes in a particularly bad way. Jeff
