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,
> 


Reply via email to