On Tue, Aug 3, 2021 at 1:58 PM Richard Sandiford <richard.sandif...@arm.com> wrote: > > Richard Biener <richard.guent...@gmail.com> writes: > > On Mon, Aug 2, 2021 at 1:31 PM Richard Sandiford > > <richard.sandif...@arm.com> wrote: > >> > >> Richard Biener <richard.guent...@gmail.com> writes: > >> > On Mon, Aug 2, 2021 at 12:43 PM Richard Sandiford > >> > <richard.sandif...@arm.com> wrote: > >> >> > >> >> Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > >> >> > On Fri, Jul 30, 2021 at 5:59 PM Richard Sandiford via Gcc-patches > >> >> > <gcc-patches@gcc.gnu.org> wrote: > >> >> >> > >> >> >> This patch adds a simple class for holding A/B fractions. > >> >> >> As the comments in the patch say, the class isn't designed > >> >> >> to have nice numerial properties at the extremes. > >> >> >> > >> >> >> The motivating use case was some aarch64 costing work, > >> >> >> where being able to represent fractions was much easier > >> >> >> than using single integers and avoided the rounding errors > >> >> >> that would come with using floats. (Unlike things like > >> >> >> COSTS_N_INSNS, there was no sensible constant base factor > >> >> >> that could be used.) > >> >> >> > >> >> >> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > >> >> > > >> >> > Hmm, we use the sreal type for profiles. I don't see any > >> >> > overflow/underflow > >> >> > handling in your class - I suppose you're going to use it on integer > >> >> > types > >> >> > given we're not allowed to use native FP? > >> >> > >> >> Yeah, I'm going to use it on integer types. And it's not designed > >> >> to have nice properties at extremes, including handling underflow and > >> >> overflow. > >> > > >> > So maybe assert that it doesn't? In particular nominator/denominator > >> > are prone to overflowing in fractional representations. > >> > > >> > There's the option to round or ICE. Or rather than the only option > >> > is to round (or use a more expensive arbitrary precision representation). > >> > >> Yeah, I guess we could do that, but it semes inconsistent to assert > >> for these costs and not do it for vector costs in general. I think it's > >> difficult to guarantee that there is no user input for which the current > >> vector costs overflow. And if we assert, we have to have a reason for > >> believing that no such user input exists (modulo bugs). > >> > >> E.g. vect-inner-loop-cost-factor has an upper limit of 999999, so the > >> existing code only needs a cost of 2148 to overflow “int”. > > > > I'd argue those are of course bugs. The 999999 upper bound is way > > too big given REB_BR_PROB_BASE is only 10000. But then we're now > > set up to initialize vinfo->inner_loop_cost_factor based on profile data > > (if it is reliable). > > > >> > So the question is whether the fractional behavior is better in more > >> > cases than the sreal behavior (I can easily believe it is). > >> > > >> >> I want to use it in costing code, where we already happily multiply > >> >> and add “int”-sized costs without worrying about overflow. I'll be > >> >> using uint64_t for the fractions though, just in case. :-) > >> >> > >> >> sreal doesn't help because it's still significand/exponent. That > >> >> matters > >> >> because… > >> >> > >> >> > I mean, how exactly does > >> >> > the class solve the problem of rounding errors? > >> >> > >> >> …I wanted something that represented the results exactly (barring any of > >> >> integer ops overflowing). This makes it meaningful to compare costs for > >> >> equality. It also means we can use ordered comparisons without having > >> >> to introduce a fudge factor to cope with one calculation having > >> >> different > >> >> intermediate rounding from the other. > >> > > >> > I think you're underestimating how quickly your denominator will > >> > overflow? > >> > >> Well, it depends on how you use it. :-) I agree you have to go into > >> this knowing the risks of the representation (but then I'd argue that's > >> true for floats/sreals too, if you use them for costs). > > > > Yeah, and sreals handle overflow/underflow in a well-defined way because > > profile info tends to be crap ;) > > > >> > So I suppose all factors of all possible denominators are known, in fact > >> > whats your main source for the divisions? The VF? > >> > >> Yeah, the set of possible dominators is fixed at compile time and > >> relatively small, but not easily enumerable. The VF is one source, > >> but we also have “number of X per cycle” values. The problem with sreal > >> is that sometimes those “X per cycle” values are 3, and 1/3 is where the > >> rounding problems with floats/sreals start to come in. > >> > >> I'm fairly sure that using a uint64_t fractional representation for > >> int costs and these set of denominator values is safe. But if we > >> think that this is just too dangerous to advertise as a general > >> class within GCC, we could make it local to the aarch64 cost code > >> instead. Would that be OK? > > > > I think we should instead make its use safe, that is, simply round when > > the denominator gets too big. The gcn compute is already expensive > > and so is the division, I suppose a practical way would be to use > > uint32 for the representation and [u]int64 for the intermediate compute? > > > > One could put extra debugging that dumps to the active dumpfile > > whenever this happens as well (but likely with a editable #define, > > disabled by default). > > Hmm, that feels quite a bit more complicated than what I was hoping for > though.
Sorry ;) I think if we're introducing some generic abstraction it needs to adhere to higher correctness standards than random cost code sprinkled in the vectorizer... > Perhaps I was trying to generalise too far. For the aarch64 vector cost > code, we can make do with a fixed-point representation with a target- > specific scale factor, so I went with that instead. I tried to address > your correctness concerns by making the arithmetic saturating. > > I'll post the series (all AArch64-specific) in a sec. > > Thanks, > Richard