This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 2/4] Switch other switch expansion methods into classes.
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 <mliska@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 <mliska@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
>