On Fri, May 19, 2017 at 04:29:16PM -0700, Ian Romanick wrote:
> On 05/19/2017 10:28 AM, Nanley Chery wrote:
> > On Fri, May 19, 2017 at 06:38:03AM -0700, Ian Romanick wrote:
> >> From: Ian Romanick <ian.d.roman...@intel.com>
> >>
> >> The previous code handled everything with the general case. I noticed
> >> that every time I converted an open-coded check to use a
> >> _mesa_has_EXT_foo() function, the text size of the driver increased.
> >>
> >> Almost all extensions only care what the current context API is, and
> >> the version does not matter. Handle those using more compact checks.
> >>
> >> text data bss dec hex filename
> >> 7037675 235248 37280 7310203 6f8b7b 32-bit i965_dri.so before
> >> 7034307 235248 37280 7306835 6f7e53 32-bit i965_dri.so after
> >> 6679695 303400 50608 7033703 6b5367 64-bit i965_dri.so before
> >> 6676143 303400 50608 7030151 6b4587 64-bit i965_dri.so after
> >
> > Hi Ian,
> >
> > I wrote a patch some time ago that reduces the cost of the extension
> > checks by a lot more with less code. The only thing I think may need
> > addressing is endianness. Would you consider using it instead if I
> > reworked it and sent it out to the list? You can find it here:
> > https://cgit.freedesktop.org/~nchery/mesa/commit/?h=1/ext/optimize&id=a02d88eba1d3129b27d3b5e6aaa976c3ca20cf79
>
> I was not able to reproduce that result on current Mesa. I had a lot
> of trouble believing that more than 18% of our driver binary was
> extension check code. I also had a sick feeling that may have just
> been the first stage of grief talking... :) Are you able to reproduce
> your original result?
>
I'm actually not able to reproduce my results anymore. Also, after
fixing endianness with more bit shifting, I was only able to save ~400B
of .text (using your build flags). I retract my earlier statement.
> I also tried a similar patch to yours that wouldn't have endianness
> problems:
>
> #define EXT(name_str, driver_cap, gll, glc, es1, es2, ...)
> \
> static inline bool
> \
> _mesa_has_##name_str(const struct gl_context *ctx)
> \
> {
> \
> static const uint8_t ver[4] = { (uint8_t)gll, (uint8_t)es1, (uint8_t)es2,
> (uint8_t)glc }; \
> return ctx->Extensions.driver_cap &&
> \
> (ctx->Extensions.Version >= ver[ctx->API]);
> \
> }
>
> Here's what I got for all four methods:
>
> 7037675 235248 37280 7310203 6f8b7b 32-bit i965_dri.so before
> 7034307 235248 37280 7306835 6f7e53 32-bit i965_dri.so after idr
> 7038343 235248 37280 7310871 6f8e17 32-bit i965_dri.so after Nanley
> 7036271 235248 37280 7308799 6f85ff 32-bit i965_dri.so w/arrays
>
> 6679695 303400 50608 7033703 6b5367 64-bit i965_dri.so before
> 6676143 303400 50608 7030151 6b4587 64-bit i965_dri.so after idr
> 6684767 303400 50608 7038775 6b6737 64-bit i965_dri.so after Nanley
> 6678567 303400 50608 7032575 6b4eff 64-bit i965_dri.so w/arrays
>
> For reference, I build with the same flags that Fedora 23 (I think?)
> used for release builds:
>
> -O2 -g -pipe -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong
> --param=ssp-buffer-size=4 -grecord-gcc-switches
>
> And either "-m64 -mtune=generic" or "-m32 -march=i686 -mtune=atom
> -fasynchronous-unwind-tables" depending on the platform.
> --enable-debug is not passed to configure.
>
> Even if I were able to reproduce your original result, there are still
> two cases where your approach may generate more code than is necessary:
>
> 1. All of the APIs that support the extension use dummy_true. There
> are many examples of this, but I don't think there are many matching
> users of _mesa_has_XXX_foo().
>
> 2. All of the APIs support the extension in any version.
> GL_EXT_polygon_offset_clamp is an example.
>
> I think we can blend the strengths to get something even better.
>
Agreed.
-Nanley
> I missed that glsl_parser_extras.cpp has its own implementation of the
> has_XXX_foo() functions that take the API and version as explicit
> parameters. Your patch predates that change. There's room for some
> modest savings there too.
>
> I'll send a couple follow-up patches soon.
>
> > Thanks,
> > Nanley
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
