On 05/12/17 18:31, Chema Casanova wrote:
> El 05/12/17 a las 06:16, Jason Ekstrand escribió:
>> A couple of notes:
>>
>>  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.
> 
> The series is almost ready to land, I have only pending to address your
> feedback about use untyped_read for reading vec3 ssbos.
> 
> The only missing explicit R-b is that " [PATCH v4 28/44] i965/fs: Use
> untyped_surface_read for 16-bit load_ssbo" and "[PATCH v4 23/44]
> i965/fs: Enables 16-bit load_ubo with sampler" i've just answered your
> review to confirm the R-b.
> 
> I expect to finish today vec3 ssbo and send the series to Jenkins before
> landing, confirm your "pending" R-b, do a last rebase to master and ask
> for a push.

I've just prepared a rebased branch with the reviewed commits ready to
land to enable VK_KHR_16bit_storage support for SSBO/UBO.

https://github.com/Igalia/mesa/tree/wip/VK_KHR_16bit_storage-v4-ubo-ssbo-to-land

As I don't have still commit access to mesa, maybe Eduardo or Alejandro
can land it for me tomorrow. But, Jason feel free to push it if you want.

Chema

>>  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.
> 
> I'm testing this patch with 16-bits and make sure whatever is needed to
> have 16-bit working.
> 
>>
>>  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.
> 
> That would be my next priority, so we would only have pending to land
> the 16-bit input/output support to finish this extension.
> 
> Chema
> 
>> On Wed, Nov 29, 2017 at 6:07 PM, Jose Maria Casanova Crespo
>> <jmcasan...@igalia.com <mailto:jmcasan...@igalia.com>> wrote:
>>
>>     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:
>>
>>     https://github.com/Igalia/mesa/tree/wip/VK_KHR_16bit_storage-rc4
>>     <https://github.com/Igalia/mesa/tree/wip/VK_KHR_16bit_storage-rc4>
>>
>>     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]
>>     https://lists.freedesktop.org/archives/mesa-dev/2017-July/162791.html
>>     <https://lists.freedesktop.org/archives/mesa-dev/2017-July/162791.html>
>>     [2]
>>     https://lists.freedesktop.org/archives/mesa-dev/2017-August/167455.html
>>     <https://lists.freedesktop.org/archives/mesa-dev/2017-August/167455.html>
>>     [3]
>>     https://lists.freedesktop.org/archives/mesa-dev/2017-October/172557.html
>>     
>> <https://lists.freedesktop.org/archives/mesa-dev/2017-October/172557.html>
>>
>>     CC: Jason Ekstrand <ja...@jlekstrand.net
>>     <mailto:ja...@jlekstrand.net>>
>>     CC: Topi Pohjolainen <topi.pohjolai...@gmail.com
>>     <mailto:topi.pohjolai...@gmail.com>>
>>     CC: Matt Turner <matts...@gmail.com <mailto:matts...@gmail.com>>
>>
>>     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
>>
>>
>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to