On Sunday, February 08, 2015 02:52:40 PM Francisco Jerez wrote: > Kenneth Graunke <kenn...@whitecape.org> writes: [snip] > > Wow, I don't like the name of that macro at all. I would expect it to > > do the well known mathematical function, ceil(). It doesn't. > > > It does, well, sort of... I'm not sure who came up with that macro, but > it seems that what the author had in mind was indeed taking the ceiling > of the fraction given by p/q for CEILING(p, q). > > > In most places in the driver, we do: > > > > ALIGN(value, 32) / 32; > > > > which would be my preference. I definitely dislike the old open coded > > (val + 31) / 32 business. > > > That only works for powers of two, and it still forces you to duplicate > the denominator -- in this specific case it's okay but it starts to get > annoying when the denominator is longer than two keystrokes. How about > we rename CEILING() to DIV_ROUND_UP() as the kernel calls it?
Good point. I like that suggestion. > > > Beyond the style...Matt put MAX2 around the whole expression, and you > > put it around the inside. Those might differ - is one more correct than > > the other? > > It can only make a difference for type_sz(x) == 0 or for type_sz(x) > larger than the unit regs_written is expressed in. So right now they > are effectively equivalent but they wouldn't be if at some point we > wanted to keep track of sub-register (e.g. scalar) writes, if we ever do > that we'll have to take the max of width * stride only. Let's rename CEILING to DIV_ROUND_UP and go with your patch. With that change, Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
