<div dir="ltr"><div dir="ltr">I just sent one more, "nir/lower_io: Add shared to get_io_offset_src" that's required for the pass to apply properly to shared vairables.</div><div dir="ltr"> </div><div>Do we have any testing of shared variables with anything other than 32 bits? Do we even test 64 bits? I'm begining to think that there are basically zero tests for anything other than 32-bit. :( </div></div> <div class="gmail_quote"><div dir="ltr">On Wed, Nov 14, 2018 at 5:48 AM Samuel Iglesias Gonsálvez <<a href="mailto:siglesias@igalia.com">siglesias@igalia.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thanks a lot for this work. Patches 1-7 are, Reviewed-by: Samuel Iglesias Gonsálvez <<a href="mailto:siglesias@igalia.com" target="_blank">siglesias@igalia.com</a>> I will review patch 8 later, probably tomorrow. Sam On 14/11/2018 00:23, Jason Ekstrand wrote: > In order to properly do all the different kinds of SSBO and SLM writes that > we have in GL and Vulkan, we have to do some lowering. The hardware > doesn't have instructions for writing a N-bit vecM with an arbitrary > write-mask. Instead, we have byte scattered messages which work on a > scalar byte, word, or dword at an unaligned address and untyped surface > messages which work on a 32-bit vecN. All SSBO and SLM access has to be > lowered to one of these two things. > > Previously we did this in the back-end and had separate copies for fs and > vec4. This works but it was fairly heavily tied to the fs_surface_builder > and the way we emit typed load/store ops. I've been interested in wiring > up the A64 messages for doing "global" reads and writes and they will need > exactly the same lowering but I'm not at all convinced I want to shove them > through the same emit_untyped_read/write helpers we have today. In any > case, this lets us share code between vec4 and fs and I think the > implementation is over-all cleaner for it. This series has a few other > advantages beyond just code sharing: > > 1) The new splitting code acts on ranges of bytes and is able to combine > loads/stores in more cases than the old code could. For example, an > indirect u8vec3 load is now just a single dword load where we throw > away the last 16 bits. Another example is that a u16vec4 write with a > YZ writemask is now written with a single unaligned dword store. > > 2) OpBitcast in SPIR-V now works correctly on 8-bit types. > > 3) Writes to 8 and 16-bit shared variables should now work. > > Cc: Samuel Iglesias Gonsálvez <<a href="mailto:siglesias@igalia.com" target="_blank">siglesias@igalia.com</a>> > Cc: Jose Maria Casanova Crespo <<a href="mailto:jmcasanova@igalia.com" target="_blank">jmcasanova@igalia.com</a>> > > Jason Ekstrand (8): > nir/lower_alu_to_scalar: Don't try to lower unpack_32_2x16 > nir/builder: Assert that intN_t immediates fit > nir/builder: Add iadd_imm and imul_imm helpers > nir/builder: Add a nir_pack/unpack/bitcast helpers > nir/spirv: Force 32-bit for UBO and SSBO Booleans > nir/glsl: Force 32-bit for UBO and SSBO Booleans > nir: Add alignment parameters to SSBO, UBO, and shared access > intel/compiler: Lower SSBO and shared loads/stores in NIR > > src/compiler/glsl/glsl_to_nir.cpp | 31 +- > src/compiler/nir/nir.h | 41 ++ > src/compiler/nir/nir_builder.h | 142 +++++++ > src/compiler/nir/nir_intrinsics.py | 26 +- > src/compiler/nir/nir_lower_alu_to_scalar.c | 1 + > src/compiler/nir/nir_lower_atomics_to_ssbo.c | 4 + > src/compiler/nir/nir_lower_io.c | 5 +- > src/compiler/nir/nir_print.c | 2 + > src/compiler/spirv/spirv_to_nir.c | 2 + > src/compiler/spirv/vtn_alu.c | 101 ++--- > src/compiler/spirv/vtn_variables.c | 30 +- > src/intel/Makefile.sources | 1 + > src/intel/compiler/brw_fs_nir.cpp | 381 ++++-------------- > src/intel/compiler/brw_nir.c | 2 + > src/intel/compiler/brw_nir.h | 2 + > .../brw_nir_lower_mem_access_bit_sizes.c | 313 ++++++++++++++ > src/intel/compiler/brw_vec4_nir.cpp | 126 +----- > src/intel/compiler/meson.build | 1 + > 18 files changed, 702 insertions(+), 509 deletions(-) > create mode 100644 src/intel/compiler/brw_nir_lower_mem_access_bit_sizes.c > </blockquote></div>