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