[Mesa-dev] [PATCH v2 00/30] Finishing arb_gpu_shader_fp64 support to the i965 scalar backend

Mon May 16 09:04:37 UTC 2016

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 13/05/16 11:56, Iago Toral wrote:
> On Thu, 2016-05-12 at 13:35 +0200, Samuel Iglesias Gonsálvez
> wrote:
>> Hi,
>> 
>> this version includes all the feedback received to v1 plus a few
>> new patches (22-27) that deal with 64bit URB read/writes, which
>> was missing in v1. Below is a list of patches that still need to
>> get the Rb:
>> 
>> [PATCH v2 02/30] i965/fs: Fix propagation of copies with strided
>> source. [PATCH v2 05/30] i965/fs: Simplify and fix register
>> offset calculation [PATCH v2 06/30] i965/fs: Reindent register
>> offset calculation of [PATCH v2 07/30] i965/fs: fix copy
>> propagation of partially invalidated [PATCH v2 11/30] i965/fs:
>> add shuffle_32bit_load_result_to_64bit_data [PATCH v2 14/30]
>> i965/fs: fix pull constant load component selection for [PATCH v2
>> 18/30] i965/fs: support doubles with SSBO loads [PATCH v2 19/30]
>> i965/fs: add shuffle_64bit_data_for_32bit_write helper [PATCH v2
>> 20/30] i965/fs: support doubles with ssbo stores [PATCH v2 21/30]
>> i965/fs: support doubles with shared variable stores [PATCH v2
>> 22/30] i965/vec4: handle doubles in type_size_vec4() [PATCH v2
>> 23/30] i965/fs: fix number of output components for doubles 
>> [PATCH v2 24/30] i965/fs: fix nir_intrinsic_store_output for
>> doubles [PATCH v2 25/30] i965/tcs/scalar: fix load input for
>> doubles [PATCH v2 26/30] i965/tcs/scalar: fix store output for
>> doubles [PATCH v2 27/30] i965/tes/scalar: Fix load input for
>> doubles
> 
> I've just sent a v3 for patches 19 and 21. The former gets rid of
> the temporary like Curro suggested since in this case we really
> don't want to do the shuffling in-place. The latter fixes a related
> bug where we were doing in-place shuffling before a write which we
> shouldn't.
> 
> I think we have addressed all the other comments too, including
> moving the shuffling functions to brw_fs_nir.cpp. I also went ahead
> and made the do_untyped_vector_read helper static to brw_fs_nir.cpp
> (instead of a fs_visitor method) since Curro's reasoning for the
> shuffling functions applies to this helper just as much.
> 
> All these changes have been merged in our 
> i965-fp64-scalar-backend-part2-to-push branch for review /
> testing.
> 
> I think that at this point we only need the thumbs-up for those two
> v3 patches and see if Curro has more feedback since I believe he
> did not have time to go through all the patches yet. If Curro does
> not find anything major we should be able to land this tomorrow.
> 

After addressing Curro's feedback (thanks Curro!), I pushed these
patches to master! \o/

We plan to work on a follow-up patch series to improve some code but,
at least, we have arb_gpu_shader_fp64 support for i965's scalar
backend in master, ready for next Mesa release :-)

I would like to say thank you to Connor, Jason, Curro, Kenneth,
Jordan, Topi, Eric, Rob... and many others (sorry if I missed you) for
your help, reviews, suggestions, advices and patience for having this
extension ready on time.

Thanks again,

Sam

>> There is still some discussion on going about where to put the 
>> shuffling functions but it does not make sense to postpone review
>> of v2 because of that, so for now we kept them in brw_fs.cpp and
>> if we finally agree to move them to brw_fs_nir.cpp we will do
>> that before pushing.
>> 
>> We have not observed any piglit regressions in ILK, SNB, IVB,
>> HSW, BDW or SKL compared against master's ba3f0b6.
>> 
>> This series enables fp64 for gen8+ only and requires scalar GS,
>> TCS and TES so these gens can do fp64 in these stages via the
>> scalar backend, as the vec4 backend is not ready yet. Support to
>> enable the scalar backend by default for all 3 stages has already
>> landed in master so we should be all set in this regard.
>> 
>> As usual, a branch with the series is available for testing
>> here: $ git clone -b i965-fp64-scalar-backend-part2-to-push
>> https://github.com/Igalia/mesa.git
>> 
>> All the new fp64 tests we wrote have also landed in piglit,
>> except for patch [0]. We have a branch available with that test
>> included here:
>> 
>> $ git clone -b arb_gpu_shader_fp64
>> https://github.com/Igalia/piglit.git
>> 
>> Thanks,
>> 
>> Sam
>> 
>> [0]
>> https://lists.freedesktop.org/archives/piglit/2016-May/019761.html
>>
>>
>> 
Francisco Jerez (5):
>> i965/fs: Fix propagation of copies with strided source. i965/fs:
>> Simplify and fix register offset calculation of 
>> try_copy_propagate(). i965/fs: Reindent register offset
>> calculation of try_copy_propagate(). i965/fs: Stop using the
>> LOAD_PAYLOAD instruction in lower_simd_width. i965/fs: Fix and
>> document component().
>> 
>> Iago Toral Quiroga (25): i965/fs: fix subreg_offset overflow in
>> byte_offset() i965/fs: Fix copy propagation of load payload for
>> double operands i965/fs: disallow type change in copy-propagation
>> if types have different sizes i965/fs: fix copy propagation of
>> partially invalidated entries i965/fs: fix copy propagation from
>> load payload i965/fs: fix copy/constant propagation regioning
>> checks i965/fs: add shuffle_32bit_load_result_to_64bit_data
>> helper i965/fs: Fix fs_visitor::VARYING_PULL_CONSTANT_LOAD for
>> doubles i965/fs: fix pull constant load component selection for
>> doubles i965/fs: support doubles with UBO loads i965/fs: Add
>> do_untyped_vector_read helper i965/fs: support double with shared
>> variable loads i965/fs: support doubles with SSBO loads i965/fs:
>> add shuffle_64bit_data_for_32bit_write helper i965/fs: support
>> doubles with ssbo stores i965/fs: support doubles with shared
>> variable stores i965/vec4: handle doubles in type_size_vec4() 
>> i965/fs: fix number of output components for doubles i965/fs: fix
>> nir_intrinsic_store_output for doubles i965/tcs/scalar: fix load
>> input for doubles i965/tcs/scalar: fix store output for doubles 
>> i965/tes/scalar: Fix load input for doubles i965: Enable
>> ARB_gpu_shader_fp64 for gen8+ docs: Mark ARB_gpu_shader_fp64 as
>> done for i965/gen8+ i965: Expose OpenGL 4.0 for gen8+
>> 
>> docs/GL3.txt                                       |   2 +- 
>> src/mesa/drivers/dri/i965/brw_fs.cpp               | 173
>> ++++++-- src/mesa/drivers/dri/i965/brw_fs.h                 |  16
>> + .../drivers/dri/i965/brw_fs_copy_propagation.cpp   | 136
>> +++--- src/mesa/drivers/dri/i965/brw_fs_nir.cpp           | 459
>> +++++++++++++++++---- src/mesa/drivers/dri/i965/brw_ir_fs.h
>> |  17 +- src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp     |   9
>> +- src/mesa/drivers/dri/i965/intel_extensions.c       |   5 +- 
>> src/mesa/drivers/dri/i965/intel_screen.c           |   2 +- 9
>> files changed, 621 insertions(+), 198 deletions(-)
>> 
> 
> 
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJXOY0lAAoJEH/0ujLxfcNDrl4P+wcqjj41YZdFPteSN19B62Ce
9+hiBXfALl5rWAXBXYQ8FjAydVZQu5rzdhZJJMTHkf0pLhHK6VQozwEnW3H8XGQh
mEy+AZoh3cYxNm2U7VewzGhd++UwysyL4IgHvZN7aJTRApSfDYpIyXy51OY4jjip
bA996rqX76tsu2jCzduZTAT1mevZ32GZ/a5/N773vInFlNjowiH6ojj3BZUKk077
L+OxIBLMax0Jazh6iFErDUoZTYfjLdvsrRpMB+g/EBkSDxWcyHZVL7mdtgBlBhbY
2SAuajOIl2CUJyi7EvuJDtSQOny0AAxWeEDrPWFam30b25JKDfb5TA8AWwLeS4aD
kYxOt1bWfo2RiESvF7MFI+3p5zaglwQ9nAi4XlioqjPaunkOeVDodQcGINDcKQ7b
wCFzRFJmckubOlHE0HrzU0ERKGxRkerOT+txRMZQM7IyicT3o7YTTxMIpMzNY10D
b77UcB5E2BDQr80sGS4br2TcsJZ8amVndkCTT4x7lWO7+XE7s0BCf73Hi5nszFzG
B4rsLBDjLrRasgj3cX3m9huu4UfPCdopuGPyDp96t/A6W1s0G0bn2rpqWZAswzHg
crEgdOSr/G1ZtyZVsoxu/XMqfz5MumDMbdeusPnloQ+ejc6sGDQEof7NSaOVG+nO
xQ8QUE2/mCPLUB6ZoJHD
=xJNh
-----END PGP SIGNATURE-----