[Mesa-dev] i965 implementation of the ARB_shader_image_load_store built-ins. (v2)
currojerez at riseup.net
Tue May 5 14:17:14 PDT 2015
Kenneth Graunke <kenneth at whitecape.org> writes:
> On Saturday, May 02, 2015 06:29:27 PM Francisco Jerez wrote:
>> This v2 is motivated by the less than enthusiastic reception of my
>> last two series implementing the image load, store and atomic
>> intrinsics in the i965 back-end. It substitutes both.
>> The built-ins are now implemented in the scalar back-end only, what
>> means that IVB and HSW are no longer able to expose image support for
>> the VS and GS stages -- behaviour acceptable according to the
>> specification. BDW and up should still be able to expose images in
>> the VS stage, and possibly in the GS stage at some point too. CS
>> images can be supported on all gens (that support CS that is). The
>> benefit is that the FS/VEC4 abstraction becomes unnecessary and the
>> overall set of changes amounts to less code.
>> It can be found in the following branch along with its dependencies:
>> src/mesa/drivers/dri/i965/Makefile.sources | 4 +
>> src/mesa/drivers/dri/i965/brw_context.c | 12 ++
>> src/mesa/drivers/dri/i965/brw_fs.cpp | 43 ++++-
>> src/mesa/drivers/dri/i965/brw_fs.h | 19 +-
>> src/mesa/drivers/dri/i965/brw_fs_builder.h | 617 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 8 +-
>> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 56 ++++--
>> src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp | 1144 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> src/mesa/drivers/dri/i965/brw_fs_surface_builder.h | 233 ++++++++++++++++++++++++
>> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 466 ++++++++++++++++++++++++++++-------------------
>> src/mesa/drivers/dri/i965/brw_ir_fs.h | 56 ++++++
>> src/mesa/drivers/dri/i965/brw_ir_vec4.h | 55 ++++++
>> src/mesa/drivers/dri/i965/brw_shader.cpp | 32 ++++
>> src/mesa/drivers/dri/i965/brw_shader.h | 6 +
>> src/mesa/drivers/dri/i965/brw_vec4.cpp | 10 +
>> src/mesa/drivers/dri/i965/brw_vec4.h | 6 +
>> src/mesa/drivers/dri/i965/brw_vec4_builder.h | 579 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 8 +-
>> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 179 ++++++++++--------
>> src/mesa/drivers/dri/i965/intel_extensions.c | 2 +
>> 20 files changed, 3224 insertions(+), 311 deletions(-)
>> [PATCH 01/29] i965/fs: Have component() set the register stride to zero.
>> [PATCH 02/29] i965: Add register constructors taking an backend_reg as argument.
>> [PATCH 03/29] i965: Define consistent interface to disable control flow execution masking.
>> [PATCH 04/29] i965: Define consistent interface to predicate an instruction.
>> [PATCH 05/29] i965: Define consistent interface to enable instruction conditional modifiers.
>> [PATCH 06/29] i965: Define consistent interface to enable instruction result saturation.
>> [PATCH 07/29] i965/fs: Introduce FS IR builder.
>> [PATCH 08/29] i965/vec4: Introduce VEC4 IR builder.
>> [PATCH 09/29] i965: Define the setup_vector_uniform_values() backend_visitor interface.
>> [PATCH 10/29] i965: Add support for handling image uniforms to the GLSL IR visitors.
>> [PATCH 11/29] i965: Add a visitor method to extract the result of a visit.
>> [PATCH 12/29] i965/fs: Obtain atomic counter locations by recursing through the visitor.
>> [PATCH 13/29] i965/vec4: Obtain atomic counter locations by recursing through the visitor.
>> [PATCH 14/29] i965: Lift the constness restriction on surface indices passed to untyped ops.
>> [PATCH 15/29] i965/fs: Import array utils for the surface message builder.
>> [PATCH 16/29] i965/fs: Import helpers to convert vectors into arrays and back.
>> [PATCH 17/29] i965/fs: Import surface message builder functions.
>> [PATCH 18/29] i965/fs: Import image access validity checks.
>> [PATCH 19/29] i965/fs: Import image memory offset calculation code.
>> [PATCH 20/29] i965/fs: Import image format metadata queries.
>> [PATCH 21/29] i965/fs: Import image format conversion primitives.
>> [PATCH 22/29] i965/fs: Implement image load, store and atomic.
>> [PATCH 23/29] i965/fs: Revisit GLSL IR atomic counter intrinsic translation.
>> [PATCH 24/29] i965/fs: Revisit NIR atomic counter intrinsic translation.
>> [PATCH 25/29] i965/fs: Import GLSL IR image intrinsic translation code.
>> [PATCH 26/29] i965/fs: Import GLSL IR memory barrier intrinsic translation code.
>> [PATCH 27/29] i965/fs: Drop unused untyped surface read and atomic emit methods.
>> [PATCH 28/29] i965: Define implementation constants for ARB_shader_image_load_store.
>> [PATCH 29/29] i965: Expose ARB_shader_image_load_store.
> Hi Curro,
> Thanks for sending this out. I'm definitely more comfortable with v2,
> though I do still have concerns.
> At this point, NIR is the default FS backend. I believe our plan is to
> keep the GLSL IR backend for the Mesa 10.6 release, so that people can
> try INTEL_USE_NIR=0 if they encounter any regressions. But, after 10.6
> branches (later this month!), we're free to delete it entirely.
> So, I don't think it makes sense to add new features to the deprecated
> non-NIR scalar shader backend. It's going away soon. (Igalia is also
> working on vec4 NIR support - so we hope to switch that over eventually
> too. It'll probably take a while, though.) We should only do NIR.
Right, I'll send a v3 shortly adding NIR support and dropping the GLSL
IR implementation. Some of the clean-up patches that affect the GLSL IR
visitors seem nice to have though, even if they are going to be removed
at some point in the future.
> That then begs the question - could we do the format conversion and
> address calculations in a i965-specific NIR pass? It could simplify
> the backend changes. NIR also has better CSE, which could help for
> repeated image access.
I doubt that rewriting things in NIR would be of any help at this point.
We could have support for ARB_shader_image_load_store already in the
10.6 release if we refrain from reimplementing the world in the last
minute. And I doubt that an implementation in terms of NIR would have
simplified anything, it would have required a bunch of intrinsics with
driver-specific semantics, and I must admit that I find generating NIR
even more annoying than generating i965 IR directly (for an example of
what a mean see how  spends over twenty lines to calculate a simple
"a + b * c" expression).
> We may even be able to regain vec4 support once Igalia's vec4-NIR
> backend matures.
I guess we could regain vec4 support either way if we don't mess up the
design of the next SSA-based back-end IR in a way that again prevents
any useful code sharing between the scalar and vec4 back-ends...
> I like the idea of the builder refactor - having a mechanism for emitting
> code at arbitrary points makes a ton of sense. But I'm uncomfortable
> with how much code is added - and duplicated from the existing visitor
> code - given that fs_visitor is refactored to use it.
> I agree with Jason that we should discuss our goal - what things should
> look like - but frankly, I think we should hold off on large scale
> refactoring until after we delete the GLSL IR backend. There will be
> much less code to reorganize.
I agree, I wasn't planning on migrating the rest of the compiler
back-end to the builder framework until that happens.
> It may also make sense to land Skylake support first - I believe we can
> use typed surface messages in most places, which is a lot easier. We
> could land that, then add the additional complexity to support BDW/HSW.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 212 bytes
Desc: not available
More information about the mesa-dev