Kenneth Graunke <kenn...@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: >> http://cgit.freedesktop.org/~currojerez/mesa/log/?h=image-load-store-scalar >> >> 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, > Hi Ken,
> 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 [1] 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. > > --Ken [1] http://cgit.freedesktop.org/mesa/mesa/tree/src/glsl/nir/nir_lower_atomics.c#n84
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
