On Fri, May 10, 2024 at 4:25 AM HAO CHEN GUI <guih...@linux.ibm.com> wrote: > > 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?
But if targets return sth < COSTS_N_INSNS (1) but > 0 this is now no longer meaningful. So shouldn't it instead be return cost > 0 ? cost : 1; ? Alternatively returning fractions of COSTS_N_INSNS (1) from set_src_cost is invalid and thus the target is at fault (I do think that making zero the unknown value is quite bad since that makes it impossible to have zero as cost represented). It seems the check is to aovid pattern_cost return zero (unknown), so the comment holds to pattern_cost the same (it returns an 'int' so the better exceptional value would have been -1, avoiding the compare). Richard. > 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 > 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
