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 <[email protected]>
>>
>> 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 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.
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
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
