[Mesa-dev] [PATCH 09/32] i965/fs: Fix fs_inst::regs_written calculation for instructions with scalar dst.

[Kenneth Graunke]

On Sunday, February 08, 2015 02:52:40 PM Francisco Jerez wrote:
> Kenneth Graunke <kenneth at 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 <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150208/f233f971/attachment-0001.sig>