| Message ID | 20190802151028.15590-5-rdapp@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Improve icvt "convert multiple" | expand |
Robin Dapp <rdapp@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
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) { 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); + 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; 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;