[Mesa-dev] i965 implementation of the ARB_shader_image_load_store built-ins. (v4)

[Jason Ekstrand]

On Fri, Jul 31, 2015 at 5:20 AM, Francisco Jerez <currojerez at riseup.net> wrote:
> Jason Ekstrand <jason at jlekstrand.net> writes:
>
>> Curro,
>> What are we still wainting on for the image_load_store extension?  I
>> think I've given you R-B's on all but one or two of the compiler
>> patches.  Is the state setup stuff reviewed?  Is there anything else
>> that needs review?
>
> I've made a list of the patches still pending review that are required
> for us to expose ARB_shader_image_load_store -- They're not that many so
> I hope we can make it before the feature freeze ;).  From this series:
>
>  - i965/fs: Import code to transform image coordinates into surface coordinates.
>  - i965/fs: Implement image load, store and atomic.
>  - i965: Teach type_size() about the size of an image uniform.
>  - i965: Implement logic to set up and upload an image uniform.
>
> From the state upload series [1]:
>  - i965: Implement surface state set-up for shader images.
>  - i965: Define and initialize image parameter structure.
>  - i965: Reserve enough parameter entries for all image uniforms used in the program.
>  - i965: Hook up image state upload.
>
> From a two-patch series of SKL-specific fixes [2]:
>  - i965: Fix brw_memory_barrier() for SKL.
>  - i965: Add SKL support to brw_miptree_get_horizontal_slice_pitch().

I think I've now either reviewed or comment on all of those.  Should
be down to about 2 or 3 patches now.
--Jason

> [1] http://lists.freedesktop.org/archives/mesa-dev/2015-February/076392.html
> [2] http://lists.freedesktop.org/archives/mesa-dev/2015-May/084141.html
>
>> --Jason
>>
>> On Thu, Jul 23, 2015 at 6:58 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>>> Jason Ekstrand <jason at jlekstrand.net> writes:
>>>
>>>> *whew*, I've made it through the entire series...
>>>>
>>> Thanks!
>>>
>>>> On Tue, Jul 21, 2015 at 9:38 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>>>>> Another resend of the i965 compiler-related changes for
>>>>> ARB_shader_image_load_store, reworked to make use of the SIMD lowering
>>>>> infrastructure introduced in a previous series [1].  For a
>>>>> self-contained branch in testable form see [2].
>>>>>
>>>>> [1] http://lists.freedesktop.org/archives/mesa-dev/2015-July/089009.html
>>>>> [2] http://cgit.freedesktop.org/~currojerez/mesa/log/?h=image-load-store-lower
>>>>>
>>>>> [PATCH 01/20] i965/fs: Define logical typed and untyped surface opcodes.
>>>>> [PATCH 02/20] i965/fs: Hook up SIMD lowering to unroll surface instructions of unsupported width.
>>>>> [PATCH 03/20] i965/fs: Implement lowering of logical surface instructions.
>>>>> [PATCH 04/20] i965/fs: Handle zero-size allocations in fs_builder::vgrf().
>>>>> [PATCH 05/20] i965/fs: Import surface message builder helper functions.
>>>>> [PATCH 06/20] i965/fs: Import image access validity checks.
>>>>
>>>> The above are all
>>>>
>>>> Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>
>>>>
>>>>> [PATCH 07/20] i965/fs: Import image memory offset calculation code.
>>>>
>>>> As I said in the e-mail, this needs a *lot* more comments.  I'll
>>>> re-try the review once you've actually explained what it's doing.
>>>>
>>>>> [PATCH 08/20] i965/fs: Import image format metadata queries.
>>>>
>>>> Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>
>>>>
>>>>> [PATCH 09/20] i965/fs: Import image format conversion primitives.
>>>>
>>>> I had some minor concerns here.
>>>>
>>>>> [PATCH 10/20] i965/fs: Implement image load, store and atomic.
>>>>
>>>> This looks pretty good.  However, I had some concerns about things
>>>> getting out-of-sync with state setup that I'd like to see addressed.
>>>>
>>>>> [PATCH 11/20] i965/fs: Revisit NIR atomic counter intrinsic translation.
>>>>> [PATCH 12/20] i965/fs: Drop unused untyped surface read and atomic emit methods.
>>>>
>>>> Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>
>>>>
>>>>> [PATCH 13/20] i965: Teach type_size() about the size of an image uniform.
>>>>> [PATCH 14/20] i965: Implement logic to set up and upload an image uniform.
>>>>
>>>> I'm not particularly familiar with how mesa's uniform setup works.  Ken?
>>>>
>>>>> [PATCH 15/20] i965/fs: Don't overwrite fs_visitor::uniforms and ::param_size during the SIMD16 run.
>>>>> [PATCH 16/20] i965/fs: Execute nir_setup_uniforms, _inputs and _outputs unconditionally.
>>>>
>>>> Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>
>>>>
>>>>> [PATCH 17/20] i965/fs: Handle image uniforms in NIR programs.
>>>>
>>>> Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>
>>>>
>>>>> [PATCH 18/20] i965/fs: Translate image load, store and atomic NIR intrinsics.
>>>>
>>>> I had some comments on this one regarding encapsulation that I'd like
>>>> to see addressed.
>>>>
>>>>> [PATCH 19/20] i965/fs: Translate memory barrier NIR intrinsics.
>>>>
>>>> Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>
>>>>
>>>>> [PATCH 20/20] i965: Expose ARB_shader_image_load_store.
>>>>
>>>> I'm going to wait until the rest of it is reviewed to add mine to that
>>>> little patch. :-)
>>>>
>>>>> _______________________________________________
>>>>> mesa-dev mailing list
>>>>> mesa-dev at lists.freedesktop.org
>>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev