Re: [Mesa-dev] [PATCH v2 0/8] anv: VK_KHR_16bit_storage enabling SSBO/UBO/PushConstant

2018-02-28 Thread Chema Casanova
On 28/02/18 18:02, Jason Ekstrand wrote:
> I think all the interesting patches are reviewed now.  All the boring
> "turn stuff on" patches are
> 
> Reviewed-by: Jason Ekstrand  >

Thanks a lot for the quick review of the series. One step less to enable
the VK_KHR_16bit_storage features completely.

Chema

> On Tue, Feb 27, 2018 at 5:27 AM, Jose Maria Casanova Crespo
> > wrote:
> 
> This v2 series includes several fixes to allow enabling the
> VK_KHR_16bit_storage
> features in ANV that have already landed but are currently disabled.
> 
> Main differences with V1 [1] are:
> 
>    * Now UBO/SSBO padding for buffers size not multiple of 4 [1/8]
> is done
>      in isl and the calculus to get the original buffer size before
>      padding is done in the backend.
>    * Now load_ubo/ssbo [3/8] at constant offsets use
> untyped_surface_read
>      in all cases. A new patch [2/8] enables the
>      shuffle_32bit_load_result_to_16bit_data to skip components.
>    * vtn_type_block_size [6/8] has been simplified using
> glsl_get_bit_size.
> 
> Patches 2 and 3 and the re-enablement of features 5 and 8 are the
> ones with
> pending review.
> 
> The series includes the following fixes:
> 
>    * [1] Fixes issues in UBO/SSBO support when buffer size is not
> multiple
>      of 4. Patch adds a padding so the size will always include the
> last DWord
>      completelly. For unsized SSBO arrays there are some bits
> arithmetic to
>      allow recalculating the original size without the padding to
> calculate the
>      number of elements correctly.
>    * [2-4] Fixes the behaviour when VK_KHR_relaxed_block_layout is
> enabled, when
>      we can not guarantee that the surface read/write offsets are
> multiple of 4.
>    * [5] Enables VK_KHR_16bit_storage for SSBO and UBO.
>    * [6-8] Enables 16-bit push constants removing/changing asserts
> that don't
>      apply anymore to 16-bit case and a fix in the calculus os the
> size to be
>      read.
> 
> To catch this issues several new tests were developed and they will
> be included
> upstream to VK-GL-CTS.
> 
> This new version of this fixup series creates some conflicts in the
> re-submitted
> V5 series with the 16-bit Input/Output support that is still pending
> of review.
> An updated version including both series has been force-pushed at [2]
> 
> [1]
> https://lists.freedesktop.org/archives/mesa-dev/2018-February/186544.html
> 
> 
> [2] https://github.com/Igalia/mesa/tree/wip/VK_KHR_16bit_storage-rc5
> 
> 
> Cc: Jason Ekstrand  >
> 
> Jose Maria Casanova Crespo (8):
>   isl/i965/fs: SSBO/UBO buffers need size padding if not multiple of
>     32-bit
>   i965/fs: shuffle_32bit_load_result_to_16bit_data now skips components
>   i965/fs: Support 16-bit do_read_vector with
>     VK_KHR_relaxed_block_layout
>   i965/fs: Support 16-bit store_ssbo with VK_KHR_relaxed_block_layout
>   anv: Enable VK_KHR_16bit_storage for SSBO and UBO
>   spirv: Calculate properly 16-bit vector sizes
>   spirv/i965/anv: Relax push constant offset assertions being 32-bit
>     aligned
>   anv: Enable VK_KHR_16bit_storage for PushConstant
> 
>  src/compiler/spirv/vtn_variables.c              |   9 +-
>  src/intel/compiler/brw_fs.cpp                   |   2 +-
>  src/intel/compiler/brw_fs.h                     |   3 +-
>  src/intel/compiler/brw_fs_nir.cpp               | 124
> ++--
>  src/intel/isl/isl_surface_state.c               |  22 -
>  src/intel/vulkan/anv_device.c                   |  18 +++-
>  src/intel/vulkan/anv_extensions.py              |   2 +-
>  src/intel/vulkan/anv_nir_lower_push_constants.c |   2 -
>  8 files changed, 137 insertions(+), 45 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


Re: [Mesa-dev] [PATCH v2 0/8] anv: VK_KHR_16bit_storage enabling SSBO/UBO/PushConstant

2018-02-28 Thread Jason Ekstrand
I think all the interesting patches are reviewed now.  All the boring "turn
stuff on" patches are

Reviewed-by: Jason Ekstrand 

On Tue, Feb 27, 2018 at 5:27 AM, Jose Maria Casanova Crespo <
jmcasan...@igalia.com> wrote:

> This v2 series includes several fixes to allow enabling the
> VK_KHR_16bit_storage
> features in ANV that have already landed but are currently disabled.
>
> Main differences with V1 [1] are:
>
>* Now UBO/SSBO padding for buffers size not multiple of 4 [1/8] is done
>  in isl and the calculus to get the original buffer size before
>  padding is done in the backend.
>* Now load_ubo/ssbo [3/8] at constant offsets use untyped_surface_read
>  in all cases. A new patch [2/8] enables the
>  shuffle_32bit_load_result_to_16bit_data to skip components.
>* vtn_type_block_size [6/8] has been simplified using glsl_get_bit_size.
>
> Patches 2 and 3 and the re-enablement of features 5 and 8 are the ones with
> pending review.
>
> The series includes the following fixes:
>
>* [1] Fixes issues in UBO/SSBO support when buffer size is not multiple
>  of 4. Patch adds a padding so the size will always include the last
> DWord
>  completelly. For unsized SSBO arrays there are some bits arithmetic to
>  allow recalculating the original size without the padding to
> calculate the
>  number of elements correctly.
>* [2-4] Fixes the behaviour when VK_KHR_relaxed_block_layout is
> enabled, when
>  we can not guarantee that the surface read/write offsets are multiple
> of 4.
>* [5] Enables VK_KHR_16bit_storage for SSBO and UBO.
>* [6-8] Enables 16-bit push constants removing/changing asserts that
> don't
>  apply anymore to 16-bit case and a fix in the calculus os the size to
> be
>  read.
>
> To catch this issues several new tests were developed and they will be
> included
> upstream to VK-GL-CTS.
>
> This new version of this fixup series creates some conflicts in the
> re-submitted
> V5 series with the 16-bit Input/Output support that is still pending of
> review.
> An updated version including both series has been force-pushed at [2]
>
> [1] https://lists.freedesktop.org/archives/mesa-dev/2018-
> February/186544.html
> [2] https://github.com/Igalia/mesa/tree/wip/VK_KHR_16bit_storage-rc5
>
> Cc: Jason Ekstrand 
>
> Jose Maria Casanova Crespo (8):
>   isl/i965/fs: SSBO/UBO buffers need size padding if not multiple of
> 32-bit
>   i965/fs: shuffle_32bit_load_result_to_16bit_data now skips components
>   i965/fs: Support 16-bit do_read_vector with
> VK_KHR_relaxed_block_layout
>   i965/fs: Support 16-bit store_ssbo with VK_KHR_relaxed_block_layout
>   anv: Enable VK_KHR_16bit_storage for SSBO and UBO
>   spirv: Calculate properly 16-bit vector sizes
>   spirv/i965/anv: Relax push constant offset assertions being 32-bit
> aligned
>   anv: Enable VK_KHR_16bit_storage for PushConstant
>
>  src/compiler/spirv/vtn_variables.c  |   9 +-
>  src/intel/compiler/brw_fs.cpp   |   2 +-
>  src/intel/compiler/brw_fs.h |   3 +-
>  src/intel/compiler/brw_fs_nir.cpp   | 124
> ++--
>  src/intel/isl/isl_surface_state.c   |  22 -
>  src/intel/vulkan/anv_device.c   |  18 +++-
>  src/intel/vulkan/anv_extensions.py  |   2 +-
>  src/intel/vulkan/anv_nir_lower_push_constants.c |   2 -
>  8 files changed, 137 insertions(+), 45 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