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

[Alejandro Piñeiro]

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 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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171206/1c754f17/attachment.sig>