[Mesa-dev] [PATCH] mesa: define nir_spirv_supported_capabilities

[Timothy Arceri]

On 07/12/17 00:23, Alejandro Piñeiro wrote:
> On 06/12/17 13:29, Pierre Moreau wrote:
>> Hello Alejandro,
>>
>> As far as I understand, nir_spirv_supported_capabilities is being filled in by
>> the driver and then fetched by the API entrypoint to check the capabilities
>> required by the SPIR-V binary given as input. And this is done regardless of
>> the input IR used by the driver, be it NIR, LLVM IR, TGSI or others. So
>> couldn’t it be just named spirv_supported_capabilities? Unless it also reflects
>> the capabilities supported by the IR being used.
> 
> Good point. spirv_supported_capabilities is probably a better name,
> although right now, it would be only used on the spirv to nir pass. I
> will not send a new version of the patch with just the renaming, but for
> anyone interested on review the commit, I will use that name.

I would be much happier with this being in mtypes.h with that name. So 
if renamed:

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

> 
>> I guess nir_spirv_supported_capabilities could be extended later on to also add
>> capabilities specific to OpenCL when clover reaches OpenCL 1.2 support (and can
>> start accepting SPIR-V binaries as input through the cl_khr_il_program
>> extension), or would it be better to have a separate one for OpenCL?
> 
> Probably it would be re-used, but I don't know the specifics of OpenCL
> to ensure 100% that.
> 
>>
>> I haven’t had time to look at the whole gl_spirv series yet, so I am sorry if
>> this is something that has already been brought and answered in that thread.
> 
> No sorries, your feedback was good and welcomed.
>>
>> Regards,
>> Pierre
>>
>> On 2017-12-06 — 09:57, Alejandro Piñeiro wrote:
>>> Until now it was part of spirv_to_nir_options. But it will be used on
>>> the implementation of ARB_gl_spirv and ARB_spirv_extensions, and added
>>> to the OpenGL context, as a way to save what SPIR-V capabilities the
>>> current OpenGL implementation supports.
>>> ---
>>>
>>> We are sending this commit in advance of a v3 of the initial gl_spirv
>>> and spirv_extensions support series. The issue is that lately there
>>> were a lot of activity on the spirv/spir_to_nir code base, and we are
>>> being fixing rebase conflicts constantly. Getting this commit on
>>> master would make things easier.
>>>
>>> FWIW, this patch is similar to one that Ian Romanick already granted
>>> Rb, but that was dropped after all the mentioned changes:
>>> https://lists.freedesktop.org/archives/mesa-dev/2017-November/178261.html
>>>
>>>   src/compiler/spirv/nir_spirv.h | 16 +++-------------
>>>   src/mesa/main/mtypes.h         | 12 ++++++++++++
>>>   2 files changed, 15 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/src/compiler/spirv/nir_spirv.h b/src/compiler/spirv/nir_spirv.h
>>> index 43ec19d5a50..113bd710a00 100644
>>> --- a/src/compiler/spirv/nir_spirv.h
>>> +++ b/src/compiler/spirv/nir_spirv.h
>>> @@ -28,7 +28,8 @@
>>>   #ifndef _NIR_SPIRV_H_
>>>   #define _NIR_SPIRV_H_
>>>   
>>> -#include "nir/nir.h"
>>> +#include "compiler/nir/nir.h"
>>> +#include "main/mtypes.h"
>>>   
>>>   #ifdef __cplusplus
>>>   extern "C" {
>>> @@ -57,18 +58,7 @@ struct spirv_to_nir_options {
>>>       */
>>>      bool lower_workgroup_access_to_offsets;
>>>   
>>> -   struct {
>>> -      bool float64;
>>> -      bool image_ms_array;
>>> -      bool tessellation;
>>> -      bool draw_parameters;
>>> -      bool image_read_without_format;
>>> -      bool image_write_without_format;
>>> -      bool int64;
>>> -      bool multiview;
>>> -      bool variable_pointers;
>>> -      bool storage_16bit;
>>> -   } caps;
>>> +   struct nir_spirv_supported_capabilities caps;
>>>   
>>>      struct {
>>>         void (*func)(void *private_data,
>>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>>> index b478f6158e2..7da05aa3ee9 100644
>>> --- a/src/mesa/main/mtypes.h
>>> +++ b/src/mesa/main/mtypes.h
>>> @@ -3579,6 +3579,18 @@ struct gl_program_constants
>>>      GLuint MaxShaderStorageBlocks;
>>>   };
>>>   
>>> +struct nir_spirv_supported_capabilities {
>>> +   bool float64;
>>> +   bool image_ms_array;
>>> +   bool tessellation;
>>> +   bool draw_parameters;
>>> +   bool image_read_without_format;
>>> +   bool image_write_without_format;
>>> +   bool int64;
>>> +   bool multiview;
>>> +   bool variable_pointers;
>>> +   bool storage_16bit;
>>> +};
>>>   
>>>   /**
>>>    * Constants which may be overridden by device driver during context creation
>>> -- 
>>> 2.11.0
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> 
> 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>