Robin Dapp <rd...@linux.ibm.com> writes: > This patch extends bb_ok_for_noce_convert_multiple_sets by a temporary > cost estimation that can be used by noce_convert_multiple_sets.
I agree it looks like an omission that we didn't do this. The patch looks OK to me (maybe independently of the rest?) bar minor things: > diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c > index 253b8a96c1a..55205cac153 100644 > --- a/gcc/ifcvt.c > +++ b/gcc/ifcvt.c > @@ -3333,11 +3333,13 @@ noce_convert_multiple_sets (struct noce_if_info > *if_info) > fewer than PARAM_MAX_RTL_IF_CONVERSION_INSNS sets. */ > > static bool > -bb_ok_for_noce_convert_multiple_sets (basic_block test_bb) > +bb_ok_for_noce_convert_multiple_sets (basic_block test_bb, unsigned *cost) The function comment should document COST. > { > rtx_insn *insn; > unsigned count = 0; > unsigned param = PARAM_VALUE (PARAM_MAX_RTL_IF_CONVERSION_INSNS); > + bool speed_p = optimize_bb_for_speed_p (test_bb); > + unsigned potential_cost = 0; > > FOR_BB_INSNS (test_bb, insn) > { > @@ -3373,9 +3375,14 @@ bb_ok_for_noce_convert_multiple_sets (basic_block > test_bb) > if (!can_conditionally_move_p (GET_MODE (dest))) > return false; > > + rtx sset = single_set (insn); This is already available as "set", unless I'm missing something. > + potential_cost += pattern_cost (sset, speed_p); > + > count++; > } > > + *cost += potential_cost; > + > /* If we would only put out one conditional move, the other strategies > this pass tries are better optimized and will be more appropriate. > Some targets want to strictly limit the number of conditional moves > @@ -3414,11 +3421,15 @@ noce_process_if_block (struct noce_if_info *if_info) > ??? For future expansion, further expand the "multiple X" rules. */ > > /* First look for multiple SETS. */ > + unsigned int mcost = if_info->original_cost; > + unsigned tmp_cost = if_info->original_cost; Very minor, but it'd be good to be consistent about the choice between unsigned and unsigned int. Maybe "old_cost" would be a better name than "tmp_cost". > if (!else_bb > && HAVE_conditional_move > && !HAVE_cc0 > - && bb_ok_for_noce_convert_multiple_sets (then_bb)) > + && bb_ok_for_noce_convert_multiple_sets (then_bb, &mcost)) > { > + /* Temporarily set the original costs to what we estimated. */ > + if_info->original_cost = mcost; > if (noce_convert_multiple_sets (if_info)) > { > if (dump_file && if_info->transform) > @@ -3427,6 +3438,8 @@ noce_process_if_block (struct noce_if_info *if_info) > return TRUE; > } > } > + /* Restore the original costs. */ > + if_info->original_cost = tmp_cost; > > bool speed_p = optimize_bb_for_speed_p (test_bb); > unsigned int then_cost = 0, else_cost = 0; I guess the save and restore only really need to be done inside the outer "if". Not that the performance difference is going to be noticeable, but maybe it would be a bit clearer to read. Thanks, Richard