| Message ID | 9b57c474-b33b-4a89-82f2-f9a33b1810df@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | rtlanal: Correct cost regularization in pattern_cost | expand |
Hi, 在 2024/5/10 20:50, Richard Biener 写道: > IMO give we're dispatching to the rtx_cost hook eventually it needs > documenting there or alternatively catching zero and adjusting its > result there. Of course cost == 0 ? 1 : cost is wrong as it makes > zero vs. one the same cost - using cost + 1 when from rtx_cost > might be more correct, at least preserving relative costs. I tested the draft patch which sets "cost > 0 ? cost + 1 : 1;". Some regression cases are found on x86. The main problems are: The cost compare with COSTS_N_INSNS (1) doesn't works any more with the patch. As all costs are added with 1, the following compare returns true when the cost is 5 but false originally. if (cost > COSTS_N_INSNS (1)) Another problem is the cost is from set_src_cost, it doesn't take dest into consideration. For example, the cost of a store "[`x']=r109:SI" is set to 1 as it only measure the cost of set_src. It seems unreasonable. IMHO, the cost less than COSTS_N_INSNS (1) is meaningful in rtx_cost calculation but unreasonable for an insn. Should the minimum cost of an insn be set to COSTS_N_INSNS (1)? Thanks Gui Haochen
diff --git a/gcc/rtlanal.cc b/gcc/rtlanal.cc index 4158a531bdd..f7b3d7d72ce 100644 --- a/gcc/rtlanal.cc +++ b/gcc/rtlanal.cc @@ -5762,7 +5762,7 @@ pattern_cost (rtx pat, bool speed) return 0; cost = set_src_cost (SET_SRC (set), GET_MODE (SET_DEST (set)), speed); - return cost > 0 ? cost : COSTS_N_INSNS (1); + return cost > COSTS_N_INSNS (1) ? cost : COSTS_N_INSNS (1); } /* Calculate the cost of a single instruction. A return value of zero
Hi, The cost return from set_src_cost might be zero. Zero for pattern_cost means unknown cost. So the regularization converts the zero to COSTS_N_INSNS (1). // pattern_cost cost = set_src_cost (SET_SRC (set), GET_MODE (SET_DEST (set)), speed); return cost > 0 ? cost : COSTS_N_INSNS (1); But if set_src_cost returns a value less than COSTS_N_INSNS (1), it's untouched and just returned by pattern_cost. Thus "zero" from set_src_cost is higher than "one" from set_src_cost. For instance, i386 returns cost "one" for zero_extend op. //ix86_rtx_costs case ZERO_EXTEND: /* The zero extensions is often completely free on x86_64, so make it as cheap as possible. */ if (TARGET_64BIT && mode == DImode && GET_MODE (XEXP (x, 0)) == SImode) *total = 1; This patch fixes the problem by converting all costs which are less than COSTS_N_INSNS (1) to COSTS_N_INSNS (1). Bootstrapped and tested on x86 and powerpc64-linux BE and LE with no regressions. Is it OK for the trunk? Thanks Gui Haochen ChangeLog rtlanal: Correct cost regularization in pattern_cost For the pattern_cost (insn_cost), the smallest known cost is COSTS_N_INSNS (1) and zero means the cost is unknown. The method calls set_src_cost which might returns 0 or a value less than COSTS_N_INSNS (1). For these cases, pattern_cost should always return COSTS_N_INSNS (1). Current regularization is wrong and a value less than COSTS_N_INSNS (1) but larger than 0 will be returned. This patch corrects it. gcc/ * rtlanal.cc (pattern_cost): Return COSTS_N_INSNS (1) when the cost is less than COSTS_N_INSNS (1). patch.diff