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
> 

Reply via email to