<div dir="ltr"><div><div><div><div>A couple of notes: </div> 1) I *think* I gave you enough reviews to land the UBO/SSBO part and the optimizations in 26-28. If reviews are still missing anywhere, please let me know. If not, let's try and get that part landed. </div> 2) I send out a patch to rewrite assign_constant_locations which I think should make it automatically handle 8 and 16-bit values as well. I'd rather do that than more special casing if everything works out ok. </div> 3) I sent out a series of patches to enable pushing of UBOs in Vulkan. If we're not careful, these will clash with 16bit storage as UBO support suddenly has to imply push constant support. That said, I"m willing to wait at least a little while before landing them to let us get 16bit push constant support sorted out. The UBO pushing patches give us a nice little performance boost but we're nowhere near a release and I don't want it blocking you. </div>--Jason <div><div><div><div> </div><div> <div><div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Nov 29, 2017 at 6:07 PM, Jose Maria Casanova Crespo <<a href="mailto:jmcasanova@igalia.com" target="_blank">jmcasanova@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello, this is the V4 series for the implementation of the SPV_KHR_16bit_storage and VK_KHR_16bit_storage extensions on the anv vulkan driver, in addition to the GLSL and NIR support needed. The original series can be found here [1], the following v2 [2] and v3 [3]. In short V4 includes the following: * Reorder the series to enable features as they are implemented, the series now enables first UBO and SSBO support, and then inputs/outputs and finally push constants. * Support the byte scattered read/write messages with different bit sizes byte/word/dword. * Refactor of the store_ssbo code and also fix stores when writemask was .yz * Uses the sampler for load_ubo avoiding the initial implementation of the series using byte_scattered_read. * Addressed all the feedback provided by Jason and Topi on v3 review. This series is also available at: <a href="https://github.com/Igalia/mesa/tree/wip/VK_KHR_16bit_storage-rc4" rel="noreferrer" target="_blank">https://github.com/Igalia/mesa/tree/wip/VK_KHR_16bit_storage-rc4</a> The objective is to start landing part of this series, all feedback has been addressed for SSBO and UBO. But for input/outputs features it will probably need another iteration as was not completely reviewed. It is also needed to define the approach for push constants issues before of after landing the support with this implementation. Patches 1-5 and 8-17 have already been reviewed. Patch 7 was already reviewed but as it has changed too much i would appreciate another review. When patches until 25 or 28 are reviewed we could land UBOs and SSBOs support. Finally an updated overview of the patches: Patches 1-2 add 16-bit float, int and uint types to GLSL. This is needed because NIR uses GLSL types internally. We use the enums already defined at AMD_gpu_shader_half_float and NV_gpu_shader extensions. Patch 2 updates mesa/st, in order to avoid warnings for types not handled on a switch. Patches 3-6 add NIR support for those new GLSL 16-bit types, conversion opcodes, and rounding modes for float to half-float conversions. Patches 7-9 add the SPIR-V (SPV_KHR_16bit_storage) to NIR support. Patches 10-12 add general 16-bit support for i965. This includes handling of new types on several general purpose methods, update/remove some asserts. Patches 14-17 add support for 32 to 16-bit conversions for i965, including rounding mode opcodes (needed for float to half-float conversions), and an optimization that removes superfluous rounding mode sets. Patches 18-21 add and use two new messages: byte scattered read and write. Those were needed because untyped surface message has a fixed 32-bit write size. Those messages are used on the 16-bit support of store SSBO, load SSBO and load shared. Patch 22 adds helpers to allow un/shuffle 16-bit components in 32-bit ones. This would be needed for following optimizations on load/store ssbo. This huck was originally in the input/outputs support, but needed a relocation because of the new order of the series. Patch 23 Enables load_ubo support for 16-bit using the sampler un-shuffling pairs 16-bit components from 32-bit. Patches 24-25 enable SPV_KHR_16bit_storage and VK_KHR_16bit_storage but only the support for SSBO and UBO on anv vulkan driver. Patches 26-28 were new patches included in V2 to improve performance reducing the use of multiple scattered messages for untyped read/write opreations. 16bit CTS tests passes without them. The other one would fix a real problem using predication (patch 27), but unfourtunately no CTS test yet catching it. Patches 29-33 implement 16-bit vertex attribute inputs support on i965. These include changes on anv. This was needed because 16-bit surface formats do implicit conversion to 32-bit. To workaround this, we override the 16-bit surface format, and use 32-bit ones. Issues related to robust buffer access have been addressed. Patch 34 implements load input and load store for all intra stage. This patch could have problems pointed by Jason related to how TCS outputs are implmemented that need more work. Patch 35-42 implements 16-bit store output support for fragment shaders on i965. Last patch enables VK_KHR_16bit for input/outputs. Patch 43-44 adds 16-bit support for push constant and enables the feature. There is still pending to work on a general solution for push constants and the mixture of different bit_sizes. [1] <a href="https://lists.freedesktop.org/archives/mesa-dev/2017-July/162791.html" rel="noreferrer" target="_blank">https://lists.freedesktop.org/archives/mesa-dev/2017-July/162791.html</a> [2] <a href="https://lists.freedesktop.org/archives/mesa-dev/2017-August/167455.html" rel="noreferrer" target="_blank">https://lists.freedesktop.org/archives/mesa-dev/2017-August/167455.html</a> [3] <a href="https://lists.freedesktop.org/archives/mesa-dev/2017-October/172557.html" rel="noreferrer" target="_blank">https://lists.freedesktop.org/archives/mesa-dev/2017-October/172557.html</a> CC: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> CC: Topi Pohjolainen <<a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a>> CC: Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> Alejandro Piñeiro (12): i965/vec4: Handle 16-bit types at type_size_xvec4 i965/fs: Remove BRW_REGISTER_TYPE_HF assert at get_exec_type i965/fs: Handle 32-bit to 16-bit conversions i965/fs: Define new shader opcode to set rounding modes i965/fs: Enable rounding mode on f2f16 ops i965/fs: Add remove_extra_rounding_modes optimization i965/fs: Use byte_scattered_write on 16-bit store_ssbo anv: Enable VK_KHR_16bit_storage for SSBO and UBO i965/fs: Predicate byte scattered writes if needed anv/pipeline: Use 32-bit surface formats for 16-bit formats anv/cmd_buffer: Add a padding to the vertex buffer i965/fs: Use half_precision data_format on 16-bit fb writes Eduardo Lima Mitev (8): glsl: Add 16-bit types mesa/st: Handle 16-bit types at st_glsl_storage_type_size() nir: Add support for 16-bit types (half float, int16 and uint16) nir: Populate conversion opcodes to 16-bit types spirv/nir: Handle 16-bit types spirv/nir: Add support for SPV_KHR_16bit_storage anv: Enable SPV_KHR_16bit_storage on gen 8+ i965/fs: Optimize 16-bit SSBO stores by packing two into a 32-bit reg Jose Maria Casanova Crespo (24): nir: Add rounding modes enum nir: Handle fp16 rounding modes at nir_type_conversion_op spirv: Enable FPRoundingMode decorator to nir operations i965: Support for 16-bit base types in helper functions i965: Add support for control register i965/fs: Add byte scattered write message and fs support i965/fs: Add byte scattered read message and fs support i965/fs: Use byte scattered read for 16-bit load_ssbo i965/fs: Helpers for un/shuffle 16-bit pairs in 32-bit components i965/fs: Enables 16-bit load_ubo with sampler i965/fs: Use untyped_surface_read for 16-bit load_ssbo compiler: Mark when input/ouput attribute at VS uses 16-bit i965/compiler: includes 16-bit vertex input i965/fs: Unpack 16-bit from 32-bit components in VS load_input i965/fs: Support 16-bit types at load_input and store_output i965/fs: Enable Render Target Write for 16-bit outputs anv: Enable VK_KHR_16bit_storage for input/output i965/fs: Include support for SEND data_format bit for Render Targets i965/disasm: Show half-precision data_format on rt_writes i965/fs: Mark 16-bit outputs on FS store_output i965/fs: 16-bit source payloads always use 1 register i965/fs: Enable 16-bit render target write on SKL and CHV i965/fs: Support push constants of 16-bit types anv: Enable VK_KHR_16bit_storage for push_constant src/compiler/builtin_type_macros.h | 26 ++ src/compiler/glsl/ast_to_hir.cpp | 3 + src/compiler/glsl/glsl_to_nir.cpp | 9 +- src/compiler/glsl/ir_clone.cpp | 3 + src/compiler/glsl/link_uniform_initializers.cpp | 3 + src/compiler/glsl/lower_buffer_access.cpp | 3 +- src/compiler/glsl_types.cpp | 93 +++++- src/compiler/glsl_types.h | 25 +- src/compiler/nir/nir.c | 6 + src/compiler/nir/nir.h | 22 +- src/compiler/nir/nir_gather_info.c | 17 +- src/compiler/nir/nir_opcodes.py | 11 +- src/compiler/nir/nir_opcodes_c.py | 17 +- src/compiler/nir/nir_split_var_copies.c | 6 + src/compiler/nir_types.cpp | 18 ++ src/compiler/nir_types.h | 8 + src/compiler/shader_info.h | 2 + src/compiler/spirv/nir_spirv.h | 1 + src/compiler/spirv/spirv_to_nir.c | 118 ++++++- src/compiler/spirv/vtn_alu.c | 35 +- src/compiler/spirv/vtn_variables.c | 21 ++ src/intel/compiler/brw_compiler.h | 1 + src/intel/compiler/brw_disasm.c | 4 + src/intel/compiler/brw_eu.h | 25 +- src/intel/compiler/brw_eu_defines.h | 36 +++ src/intel/compiler/brw_eu_emit.c | 130 +++++++- src/intel/compiler/brw_fs.cpp | 142 +++++++- src/intel/compiler/brw_fs.h | 12 + src/intel/compiler/brw_fs_copy_propagation.cpp | 8 +- src/intel/compiler/brw_fs_generator.cpp | 30 +- src/intel/compiler/brw_fs_nir.cpp | 409 ++++++++++++++++++++++-- src/intel/compiler/brw_fs_surface_builder.cpp | 23 +- src/intel/compiler/brw_fs_surface_builder.h | 14 + src/intel/compiler/brw_fs_visitor.cpp | 6 + src/intel/compiler/brw_inst.h | 1 + src/intel/compiler/brw_ir_fs.h | 3 - src/intel/compiler/brw_nir.c | 16 + src/intel/compiler/brw_reg.h | 6 + src/intel/compiler/brw_shader.cpp | 23 ++ src/intel/compiler/brw_shader.h | 7 + src/intel/compiler/brw_vec4.cpp | 1 + src/intel/compiler/brw_vec4_generator.cpp | 3 +- src/intel/compiler/brw_vec4_visitor.cpp | 3 + src/intel/vulkan/anv_device.c | 23 ++ src/intel/vulkan/anv_extensions.py | 1 + src/intel/vulkan/anv_pipeline.c | 1 + src/intel/vulkan/genX_cmd_buffer.c | 20 +- src/intel/vulkan/genX_pipeline.c | 34 ++ src/mesa/program/ir_to_mesa.cpp | 6 + src/mesa/state_tracker/st_glsl_types.cpp | 3 + 50 files changed, 1337 insertions(+), 101 deletions(-) -- 2.14.3 </blockquote></div> </div></div></div></div></div></div></div></div>