[Mesa-dev] [PATCH] util/macros: Simplify DIV_ROUND_UP() definition

[Nanley Chery]

On Thu, Dec 17, 2015 at 11:53:03AM -0800, Nanley Chery wrote:
> On Thu, Dec 17, 2015 at 11:25 AM, Matt Turner <mattst88 at gmail.com> wrote:
> 
> > On Thu, Dec 17, 2015 at 11:04 AM, Nanley Chery <nanleychery at gmail.com>
> > wrote:
> > > On Thu, Dec 17, 2015 at 12:05:46PM +0100, Glenn Kennard wrote:
> > >> On Wed, 16 Dec 2015 20:57:51 +0100, Nanley Chery <nanleychery at gmail.com>
> > wrote:
> > >>
> > >> >From: Nanley Chery <nanley.g.chery at intel.com>
> > >> >
> > >> >Commit 64880d073ab21ae1abad0c049ea2d6a1169a3cfa consolidated two
> > >> >DIV_ROUND_UP() definitions to one, but chose the more
> > >> >compute-intensive version in the process. Use the simpler version
> > >> >instead. Reduces .text size by 1360 bytes.
> > >> >
> > >> >Output of `size lib/i965_dri.so`:
> > >> >      text    data     bss     dec     hex filename
> > >> >   7850440  219264   27240 8096944  7b8cb0 lib/i965_dri.so (before)
> > >> >   7849080  219264   27240 8095584  7b8760 lib/i965_dri.so (after)
> > >> >
> > >> >Cc: Axel Davy <axel.davy at ens.fr>
> > >> >Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > >> >---
> > >> > src/util/macros.h | 2 +-
> > >> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >> >
> > >> >diff --git a/src/util/macros.h b/src/util/macros.h
> > >> >index 0c8958f..53a98a0 100644
> > >> >--- a/src/util/macros.h
> > >> >+++ b/src/util/macros.h
> > >> >@@ -211,6 +211,6 @@ do {                       \
> > >> > #endif
> > >> >/** Compute ceiling of integer quotient of A divided by B. */
> > >> >-#define DIV_ROUND_UP( A, B )  ( (A) % (B) == 0 ? (A)/(B) : (A)/(B)+1 )
> > >> >+#define DIV_ROUND_UP(A, B)  (((A) + (B) - 1) / (B))
> > >> >#endif /* UTIL_MACROS_H */
> > >>
> > >> I'll point out that these are not equivalent, one can overflow and the
> > other doesn't. You
> > >> probably want to check if the call sites have sufficient checks for
> > that before
> > >> substituting one for the other.
> > >>
> > >
> > > Good point. As I mentioned in another email, I'll leave the current
> > > macro untouched.
> >
> >
> The other email I referenced unfortunately didn't make it to the list due
> email client
> issues.
> 
> 
> > I think the chances of us relying on overflow behavior are exceedingly
> > small, and nearly all uses of DIV_ROUND_UP are in the i965 driver. I
> > think it's sufficiently safe to go ahead with the patch (but I am
> > still interested to know about your compiler and CFLAGS).
> >
> 
> My error was that I forgot to remove --enable-debug and add -02. After
> making those
> changes, my binary size also increases but by ~1.1k. Another implementation
> which
> avoids overflow increases the binary size by 2 bytes.
> 
> Correct me if I'm wrong, but aligning w/ the kernel implementation (or
> changing the
> current definition for that matter) seems like a regression more than an
> improvement.

Sorry for the previously oddly-wrapped message. I looked into this with 
Kristian, and the kernel's version does seem to produce better assembly.
It's not yet clear why the .text size increases. My current hypothesis
is that the number of no-ops increase, but that's yet to be proven.

Thanks,
Nanley