<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Dec 6, 2017 at 8:48 PM, Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com" target="_blank">tarceri@itsqueeze.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 07/12/17 00:23, Alejandro Piñeiro wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On 06/12/17 13:29, Pierre Moreau wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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. </blockquote> 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. </blockquote> I would be much happier with this being in mtypes.h with that name. So if renamed: </blockquote><div> </div><div>Ugh... I just now got around to looking at this and saw that it went in mtypes.h. Can we please move it? We've worked very hard to keep the Vulkan driver from having to pull in any GL headers and data structures and now a fairly core piece lives in mtypes.h.</div></div><div class="gmail_quote"> </div><div class="gmail_quote">--Jason </div><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Reviewed-by: Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com" target="_blank">tarceri@itsqueeze.com</a>><div class="HOEnZb"><div class="h5"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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? </blockquote> Probably it would be re-used, but I don't know the specifics of OpenCL to ensure 100% that. <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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. </blockquote> No sorries, your feedback was good and welcomed. <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Regards, Pierre On <a href="tel:2017-12-06%20%E2%80%94%2009" value="+12017120609" target="_blank">2017-12-06 — 09</a>:57, Alejandro Piñeiro wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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: <a href="https://lists.freedesktop.org/archives/mesa-dev/2017-November/178261.html" rel="noreferrer" target="_blank">https://lists.freedesktop.org/archives/mesa-dev/2017-November/178261.html</a> 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 <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></blockquote> _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote> _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </div></div></blockquote></div> </div></div>