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>