On 06/29/2017 02:51 AM, Georg-Johann Lay wrote: > On 28.06.2017 22:18, Wilco Dijkstra wrote: >> Georg-Johann Lay wrote: >> >> @@ -5300,6 +5300,9 @@ seq_cost (const rtx_insn *seq, bool spee >> set = single_set (seq); >> if (set) >> cost += set_rtx_cost (set, speed); >> + else if (INSN_P (seq) >> + && PARALLEL == GET_CODE (PATTERN (seq))) >> + cost += insn_rtx_cost (PATTERN (seq), speed); >> else >> cost++; >> >> insn_rtx_cost may return zero if it can't find something useful in the >> parallel, >> which means it may return a lower cost and even zero. Not sure whether >> this >> is important, but in eg. combine a cost of zero means infinite and so >> could have >> unintended consequences. So incrementing cost with a non-zero value >> if insn_rtx_cost == 0 would seem safer. > > Updated patch below, it just adds 1 (which is 1/4 of CONST_N_INSNS) to > avoid zero. > > >> Also why does the else do cost++ and not cost += COSTS_N_INSNS (1)? >> >> Wilco > > Dunno, I didn't change this. Maybe it's also just to escape 0. > > Johann > > gcc/ > PR middle-end/80929 > * rtlanal.c (seq_cost) [PARALLEL]: Get cost from insn_rtx_cost > instead of assuming cost of 1. So this has probably been hashed to death, but just a couple thoughts.
I think realistically one has to look at the entirety of the PARALLEL to get any reasonable costing. Summing the individual components is wrong. Taking the cost of any individual component is wrong. You might be able to make a case for the maximum cost of the individual components, but even that is almost certain to be wrong in some cases. Presumably this is part of what Segher is trying to address with his costing changes. jeff
