Re: [Mesa-dev] V2 GLSL IR & TGSI on-disk shader cache
On 17/02/17 19:26, Nicolai Hähnle wrote: On 16.02.2017 23:55, Timothy Arceri wrote: On 17/02/17 01:27, Nicolai Hähnle wrote: Hi Timothy, thank you for the update. I had a look at all the patches now, and especially the glsl parts looks basically ready to go. There are only minor comments for which I don't need a full resend of the series, and an open question on patch 22 where it would be nice to get a proper answer. Thanks! It's a relief to finally get this stuff reviewed. It's definitely good to have concrete progress. There's always the possibility that we still missed some metadata, so I wonder what your testing plan is for this. Games, obviously, but perhaps consecutive runs of piglit give useful info as well? Yes, I've been doing consecutive piglit runs throughout development. I still need to do some runs of the CTS, but at this point I'm pretty confident we should have everything. I've done a huge amount of re-factoring of the structs that store this meta-data to reduce the amount of data we need to store both in the cache and at run time, so if I missed anything it should be fairly minor and easy to fix. I'm not really sure about an authoritative source for patch 22, although I'm happy to add the group check, I think it makes some sense. Please push the patch; in its current form its better than nothing, and even the loader doesn't do fancier checks. It has my R-b. ok, thanks :) On 14.02.2017 01:52, Timothy Arceri wrote: Changes in V2: - no longer mess around storing/restoring any pointers - implemented support for compute shaders - dropped some patches only needed by i965 for now - add fallback support for shader source that is changed after its compiledi (piglit test on the list) - simplify cache enable for r600/radeonsi by unconditionally creating the cache in screen_create. Remind me how each part of the cache can be disabled? We can't really enable GLSL IR cache by itself (I guess we could enable tgis but that wouldn't make much sense). The code simply checks id ctx->Cache != NULL in various locations which means cache is enabled. There should still be some environment variable to disable the cache. R600_DEBUG=nocache, for example, since it's the driver that creates the actual disk cache structure now. oh, that would be MESA_GLSL_CACHE_DISABLE=1 which is checked when the driver calls disk_cache_create() Cheers, Nicolai Thanks, Nicolai - make glsl version (the version reported as supported by the implemenation at compile time) part of the sha1 input rather than adding mesa string to the cache object itself. This avoids fallbacks and should be more reliable. - add any drirc options as sha1 inputs - some other tidy ups suggested by Nicolai and Marek In future we probably want to check what other env vars have been set, but for now the gl/glsl version and drirc options should cover most things. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] V2 GLSL IR & TGSI on-disk shader cache
On 16.02.2017 23:55, Timothy Arceri wrote: On 17/02/17 01:27, Nicolai Hähnle wrote: Hi Timothy, thank you for the update. I had a look at all the patches now, and especially the glsl parts looks basically ready to go. There are only minor comments for which I don't need a full resend of the series, and an open question on patch 22 where it would be nice to get a proper answer. Thanks! It's a relief to finally get this stuff reviewed. It's definitely good to have concrete progress. There's always the possibility that we still missed some metadata, so I wonder what your testing plan is for this. Games, obviously, but perhaps consecutive runs of piglit give useful info as well? I'm not really sure about an authoritative source for patch 22, although I'm happy to add the group check, I think it makes some sense. Please push the patch; in its current form its better than nothing, and even the loader doesn't do fancier checks. It has my R-b. On 14.02.2017 01:52, Timothy Arceri wrote: Changes in V2: - no longer mess around storing/restoring any pointers - implemented support for compute shaders - dropped some patches only needed by i965 for now - add fallback support for shader source that is changed after its compiledi (piglit test on the list) - simplify cache enable for r600/radeonsi by unconditionally creating the cache in screen_create. Remind me how each part of the cache can be disabled? We can't really enable GLSL IR cache by itself (I guess we could enable tgis but that wouldn't make much sense). The code simply checks id ctx->Cache != NULL in various locations which means cache is enabled. There should still be some environment variable to disable the cache. R600_DEBUG=nocache, for example, since it's the driver that creates the actual disk cache structure now. Cheers, Nicolai Thanks, Nicolai - make glsl version (the version reported as supported by the implemenation at compile time) part of the sha1 input rather than adding mesa string to the cache object itself. This avoids fallbacks and should be more reliable. - add any drirc options as sha1 inputs - some other tidy ups suggested by Nicolai and Marek In future we probably want to check what other env vars have been set, but for now the gl/glsl version and drirc options should cover most things. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] V2 GLSL IR & TGSI on-disk shader cache
On 17/02/17 01:27, Nicolai Hähnle wrote: Hi Timothy, thank you for the update. I had a look at all the patches now, and especially the glsl parts looks basically ready to go. There are only minor comments for which I don't need a full resend of the series, and an open question on patch 22 where it would be nice to get a proper answer. Thanks! It's a relief to finally get this stuff reviewed. I'm not really sure about an authoritative source for patch 22, although I'm happy to add the group check, I think it makes some sense. On 14.02.2017 01:52, Timothy Arceri wrote: Changes in V2: - no longer mess around storing/restoring any pointers - implemented support for compute shaders - dropped some patches only needed by i965 for now - add fallback support for shader source that is changed after its compiledi (piglit test on the list) - simplify cache enable for r600/radeonsi by unconditionally creating the cache in screen_create. Remind me how each part of the cache can be disabled? We can't really enable GLSL IR cache by itself (I guess we could enable tgis but that wouldn't make much sense). The code simply checks id ctx->Cache != NULL in various locations which means cache is enabled. Thanks, Nicolai - make glsl version (the version reported as supported by the implemenation at compile time) part of the sha1 input rather than adding mesa string to the cache object itself. This avoids fallbacks and should be more reliable. - add any drirc options as sha1 inputs - some other tidy ups suggested by Nicolai and Marek In future we probably want to check what other env vars have been set, but for now the gl/glsl version and drirc options should cover most things. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] V2 GLSL IR & TGSI on-disk shader cache
Hi Timothy, thank you for the update. I had a look at all the patches now, and especially the glsl parts looks basically ready to go. There are only minor comments for which I don't need a full resend of the series, and an open question on patch 22 where it would be nice to get a proper answer. On 14.02.2017 01:52, Timothy Arceri wrote: Changes in V2: - no longer mess around storing/restoring any pointers - implemented support for compute shaders - dropped some patches only needed by i965 for now - add fallback support for shader source that is changed after its compiledi (piglit test on the list) - simplify cache enable for r600/radeonsi by unconditionally creating the cache in screen_create. Remind me how each part of the cache can be disabled? Thanks, Nicolai - make glsl version (the version reported as supported by the implemenation at compile time) part of the sha1 input rather than adding mesa string to the cache object itself. This avoids fallbacks and should be more reliable. - add any drirc options as sha1 inputs - some other tidy ups suggested by Nicolai and Marek In future we probably want to check what other env vars have been set, but for now the gl/glsl version and drirc options should cover most things. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] V2 GLSL IR & TGSI on-disk shader cache
Changes in V2: - no longer mess around storing/restoring any pointers - implemented support for compute shaders - dropped some patches only needed by i965 for now - add fallback support for shader source that is changed after its compiledi (piglit test on the list) - simplify cache enable for r600/radeonsi by unconditionally creating the cache in screen_create. - make glsl version (the version reported as supported by the implemenation at compile time) part of the sha1 input rather than adding mesa string to the cache object itself. This avoids fallbacks and should be more reliable. - add any drirc options as sha1 inputs - some other tidy ups suggested by Nicolai and Marek In future we probably want to check what other env vars have been set, but for now the gl/glsl version and drirc options should cover most things. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
