Bernhard,
thanks for the review.
On 18/07/12 19:32, Bernhard Reutner-Fischer wrote:
> On Tue, Jul 17, 2012 at 01:21:00PM +0200, Tom de Vries wrote:
>
>> /* The root of the compilation pass tree, once constructed. */
>> extern struct opt_pass *all_passes, *all_small_ipa_passes,
>> *all_lowering_passes,
>> Index: gcc/tree-if-switch-conversion.c
>> ===================================================================
>> --- /dev/null (new file)
>> +++ gcc/tree-if-switch-conversion.c (revision 0)
>
>> +/* Convert all trees in RANGES to TYPE. */
>> +
>> +static bool
>> +convert_ranges (tree type, VEC (range, gc) *ranges)
>> +{
>> + unsigned int ix;
>> + range r;
>> +
>> + for (ix = 0; VEC_iterate (range, ranges, ix, r); ix++)
>> + {
>> + r->low = fold_convert (type, r->low);
>> + if (TREE_TYPE (r->low) != type)
>> + return false;
>> +
>> + if (r->high == NULL_TREE)
>> + continue;
>> +
>> + r->high = fold_convert (type, r->high);
>> + if (TREE_TYPE (r->low) != type)
>
> low, not high? This is not immediately obvious to me, please explain?
>
It's not obvious because it wrong, thanks for spotting that. This will be fixed
in the next submission.
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>
>> +/* Analyze BB and store results in ifsc_info_def struct. */
>> +
>> +static void
>> +analyze_bb (basic_block bb)
>> +{
>> + gimple stmt = last_stmt (bb);
>> + tree lhs, rhs, var, constant;
>> + edge true_edge, false_edge;
>> + enum tree_code cond_code;
>> + VEC (range, gc) *ranges = NULL;
>> + unsigned int nr_stmts = 0;
>> + bool swap_edges = false;
>> + tree low, high;
>> +
>> + /* We currently only handle bbs with GIMPLE_COND. */
>> + if (!stmt || gimple_code (stmt) != GIMPLE_COND)
>> + return;
>> +
>> + cond_code = gimple_cond_code (stmt);
>> + switch (cond_code)
>> + {
>> + case EQ_EXPR:
>> + case NE_EXPR:
>> + case LE_EXPR:
>> + case GE_EXPR:
>> + break;
>> + case LT_EXPR:
>> + case GT_EXPR:
>> + /* Todo. */
>> + return;
>> + default:
>> + return;
>> + }
>> +
>> + lhs = gimple_cond_lhs (stmt);
>> + rhs = gimple_cond_rhs (stmt);
>> +
>> + /* The comparison needs to be against a constant. */
>> + if (!TREE_CONSTANT (lhs)
>> + && !TREE_CONSTANT (rhs))
>> + return;
>> +
>> + /* Normalize comparison into (var cond_code constant). */
>> + var = TREE_CONSTANT (lhs) ? rhs : lhs;
>> + constant = TREE_CONSTANT (lhs)? lhs : rhs;
>
> missing space
>
> []
This will be fixed in the next submission.
>> +/* Convert every if-chain in CHAINS into a switch statement. */
>> +
>> +static void
>> +convert_chains (VEC (if_chain, gc) *chains)
>> +{
>> + unsigned int ix;
>> + if_chain chain;
>> +
>> + if (VEC_empty (if_chain, chains))
>> + return;
>> +
>> + for (ix = 0; VEC_iterate (if_chain, chains, ix, chain); ix++)
>
> shouldn't this be FOR_EACH_VEC_ELT nowadays? Everywhere.
Same.
>> + {
>> + if (dump_file)
>> + dump_if_chain (chain);
>> +
>> + convert_if_chain_to_switch (chain);
>> +
>> + update_cfg (chain);
>> + }
>> +
>> + /* Force recalculation of dominance info. */
>> + free_dominance_info (CDI_DOMINATORS);
>> + free_dominance_info (CDI_POST_DOMINATORS);
>> +}
>
>> Index: gcc/Makefile.in
>> ===================================================================
>> --- gcc/Makefile.in (revision 189508)
>> +++ gcc/Makefile.in (working copy)
>> @@ -1391,6 +1391,7 @@ OBJS = \
>> tree-profile.o \
>> tree-scalar-evolution.o \
>> tree-sra.o \
>> + tree-if-switch-conversion.o \
>> tree-switch-conversion.o \
>> tree-ssa-address.o \
>> tree-ssa-alias.o \
>> @@ -3013,7 +3014,12 @@ tree-sra.o : tree-sra.c $(CONFIG_H) $(SY
>> $(IPA_PROP_H) $(DIAGNOSTIC_H) statistics.h $(TREE_DUMP_H) $(TIMEVAR_H) \
>> $(PARAMS_H) $(TARGET_H) $(FLAGS_H) \
>> $(DBGCNT_H) $(TREE_INLINE_H) $(GIMPLE_PRETTY_PRINT_H)
>> +tree-if-switch-conversion.o : tree-if-switch-conversion.c $(CONFIG_H) \
>> + $(SYSTEM_H) $(TREE_H) $(TM_P_H) $(TREE_FLOW_H) $(DIAGNOSTIC_H) \
>> + $(TREE_INLINE_H) $(TIMEVAR_H) $(TM_H) coretypes.h $(TREE_DUMP_H) \
>> + $(GIMPLE_H) $(TREE_PASS_H) $(FLAGS_H) $(EXPR_H) $(BASIC_BLOCK_H)
>> output.h \
>> + $(GGC_H) $(OBSTACK_H) $(PARAMS_H) $(CPPLIB_H) $(PARAMS_H)
>
> I think this list needs updating.
>
I went over the list just now and all the elements appear in other makerules as
well, so I don't see any obvious ones that should be removed. I think the
PARAMS_H is not necessary. Do you have concerns about anything else?
Thanks,
- Tom
> Nice to see if improvements, finally! :)
> TIA && cheers,
>
