On Tuesday, 2017-11-28 14:01:38 +0100, Alejandro Piñeiro wrote: > On 28/11/17 11:36, Eric Engestrom wrote: > > On Monday, 2017-11-27 13:21:31 +0200, Tapani Pälli wrote: > >> > >> On 11/15/2017 03:22 PM, Eduardo Lima Mitev wrote: > >>> From: Alejandro Piñeiro <apinhe...@igalia.com> > >>> > >>> Ideally this should be generated somehow. One option would be gather > >>> all the extension dependencies listed on the core grammar, but there > >>> would be the possibility of not including some of the extensions. > >>> > >>> Note that spirv-tools is doing it just slightly better, as it has a > >>> hardcoded list of extensions manually took from the registry, that > >>> they parse to get the enum and the to_string method (see > >>> generate_grammar_tables.py). > >>> --- > >>> src/compiler/Makefile.sources | 2 + > >>> src/compiler/spirv/spirv_extensions.c | 61 > >>> +++++++++++++++++++++++++++++++ > >>> src/compiler/spirv/spirv_extensions.h | 69 > >>> +++++++++++++++++++++++++++++++++++ > >>> 3 files changed, 132 insertions(+) > >>> create mode 100644 src/compiler/spirv/spirv_extensions.c > >>> create mode 100644 src/compiler/spirv/spirv_extensions.h > >>> > >>> diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources > >>> index 2ab8e163a26..f198456c751 100644 > >>> --- a/src/compiler/Makefile.sources > >>> +++ b/src/compiler/Makefile.sources > >>> @@ -293,6 +293,8 @@ SPIRV_FILES = \ > >>> spirv/GLSL.std.450.h \ > >>> spirv/nir_spirv.h \ > >>> spirv/spirv.h \ > >>> + spirv/spirv_extensions.c \ > >>> + spirv/spirv_extensions.h \ > >>> spirv/spirv_info.h \ > >>> spirv/spirv_to_nir.c \ > >>> spirv/vtn_alu.c \ > >>> diff --git a/src/compiler/spirv/spirv_extensions.c > >>> b/src/compiler/spirv/spirv_extensions.c > >>> new file mode 100644 > >>> index 00000000000..64d61c49d31 > >>> --- /dev/null > >>> +++ b/src/compiler/spirv/spirv_extensions.c > >>> @@ -0,0 +1,61 @@ > >>> +/* > >>> + * Copyright © 2017 Intel Corporation > >>> + * > >>> + * Permission is hereby granted, free of charge, to any person obtaining > >>> a > >>> + * copy of this software and associated documentation files (the > >>> "Software"), > >>> + * to deal in the Software without restriction, including without > >>> limitation > >>> + * the rights to use, copy, modify, merge, publish, distribute, > >>> sublicense, > >>> + * and/or sell copies of the Software, and to permit persons to whom the > >>> + * Software is furnished to do so, subject to the following conditions: > >>> + * > >>> + * The above copyright notice and this permission notice (including the > >>> next > >>> + * paragraph) shall be included in all copies or substantial portions of > >>> the > >>> + * Software. > >>> + * > >>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > >>> EXPRESS OR > >>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > >>> MERCHANTABILITY, > >>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT > >>> SHALL > >>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR > >>> OTHER > >>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > >>> ARISING > >>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > >>> DEALINGS > >>> + * IN THE SOFTWARE. > >>> + */ > >>> + > >>> +#include "spirv.h" > >>> +#include "spirv_extensions.h" > >>> + > >>> +const char * > >>> +spirv_extensions_to_string(SpvExtension ext) > >>> +{ > >>> + switch (ext) { > >>> + case SPV_AMD_shader_explicit_vertex_parameter: return > >>> "SPV_AMD_shader_explicit_vertex_parameter"; > >>> + case SPV_AMD_shader_trinary_minmax: return > >>> "SPV_AMD_shader_trinary_minmax"; > >>> + case SPV_AMD_gcn_shader: return "SPV_AMD_gcn_shader"; > >>> + case SPV_KHR_shader_ballot: return "SPV_KHR_shader_ballot"; > >>> + case SPV_AMD_shader_ballot: return "SPV_AMD_shader_ballot"; > >>> + case SPV_AMD_gpu_shader_half_float: return > >>> "SPV_AMD_gpu_shader_half_float"; > >>> + case SPV_KHR_shader_draw_parameters: return > >>> "SPV_KHR_shader_draw_parameters"; > >>> + case SPV_KHR_subgroup_vote: return "SPV_KHR_subgroup_vote"; > >>> + case SPV_KHR_16bit_storage: return "SPV_KHR_16bit_storage"; > >>> + case SPV_KHR_device_group: return "SPV_KHR_device_group"; > >>> + case SPV_KHR_multiview: return "SPV_KHR_multiview"; > >>> + case SPV_NVX_multiview_per_view_attributes: return > >>> "SPV_NVX_multiview_per_view_attributes"; > >>> + case SPV_NV_viewport_array2: return "SPV_NV_viewport_array2"; > >>> + case SPV_NV_stereo_view_rendering: return > >>> "SPV_NV_stereo_view_rendering"; > >>> + case SPV_NV_sample_mask_override_coverage: return > >>> "SPV_NV_sample_mask_override_coverage"; > >>> + case SPV_NV_geometry_shader_passthrough: return > >>> "SPV_NV_geometry_shader_passthrough"; > >>> + case SPV_AMD_texture_gather_bias_lod: return > >>> "SPV_AMD_texture_gather_bias_lod"; > >>> + case SPV_KHR_storage_buffer_storage_class: return > >>> "SPV_KHR_storage_buffer_storage_class"; > >>> + case SPV_KHR_variable_pointers: return "SPV_KHR_variable_pointers"; > >>> + case SPV_AMD_gpu_shader_int16: return "SPV_AMD_gpu_shader_int16"; > >>> + case SPV_KHR_post_depth_coverage: return > >>> "SPV_KHR_post_depth_coverage"; > >>> + case SPV_KHR_shader_atomic_counter_ops: return > >>> "SPV_KHR_shader_atomic_counter_ops"; > >>> + case SPV_EXT_shader_stencil_export: return > >>> "SPV_EXT_shader_stencil_export"; > >>> + case SPV_EXT_shader_viewport_index_layer: return > >>> "SPV_EXT_shader_viewport_index_layer"; > >>> + case SPV_AMD_shader_image_load_store_lod: return > >>> "SPV_AMD_shader_image_load_store_lod"; > >>> + case SPV_AMD_shader_fragment_mask: return > >>> "SPV_AMD_shader_fragment_mask"; > >>> + default: return "unknown"; > >> > >> This could make it a bit less noisy looking? > >> > >> --- 8< --- > >> const char * > >> spirv_extensions_to_string(SpvExtension ext) > >> { > >> #define STR(x) case x : return #x; > >> switch (ext) { > >> STR(SPV_AMD_shader_explicit_vertex_parameter); > >> ... > >> default: > >> return "unknown"; > > Actually, I would recommend removing the `default:` and moving that > > `return unknown` outside of the switch(), in order for `-Wswitch` to > > be able to do its job of warning for unhandled enum values :) > > Then it will complain due unhandled SPV_EXTENSIONS_COUNT, that we use to > count the number of SPV extensions that we know.
case SPV_EXTENSIONS_COUNT: unreachable("invalid extension"); (you don't even need a `break;` after an `unreachable()` :) > > > The `return unknown` might also deserve an assert()? > > Makes sense, although I will use a unreachable instead. > > > >> } > >> #undef STR > >> } > >> --- 8< --- > >> > >> > >>> + } > >>> + > >>> + return "unknown"; > >>> +} > >>> diff --git a/src/compiler/spirv/spirv_extensions.h > >>> b/src/compiler/spirv/spirv_extensions.h > >>> new file mode 100644 > >>> index 00000000000..54bc7f2dbe0 > >>> --- /dev/null > >>> +++ b/src/compiler/spirv/spirv_extensions.h > >>> @@ -0,0 +1,69 @@ > >>> +/* > >>> + * Copyright © 2017 Intel Corporation > >>> + * > >>> + * Permission is hereby granted, free of charge, to any person obtaining > >>> a > >>> + * copy of this software and associated documentation files (the > >>> "Software"), > >>> + * to deal in the Software without restriction, including without > >>> limitation > >>> + * the rights to use, copy, modify, merge, publish, distribute, > >>> sublicense, > >>> + * and/or sell copies of the Software, and to permit persons to whom the > >>> + * Software is furnished to do so, subject to the following conditions: > >>> + * > >>> + * The above copyright notice and this permission notice (including the > >>> next > >>> + * paragraph) shall be included in all copies or substantial portions of > >>> the > >>> + * Software. > >>> + * > >>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > >>> EXPRESS OR > >>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > >>> MERCHANTABILITY, > >>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT > >>> SHALL > >>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR > >>> OTHER > >>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > >>> ARISING > >>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > >>> DEALINGS > >>> + * IN THE SOFTWARE. > >>> + */ > >>> + > >>> +#ifndef _SPIRV_EXTENSIONS_H_ > >>> +#define _SPIRV_EXTENSIONS_H_ > >>> + > >>> +#include "compiler/nir/nir.h" > >>> + > >>> +#ifdef __cplusplus > >>> +extern "C" { > >>> +#endif > >>> + > >>> +typedef enum SpvExtension_ { > >>> + SPV_AMD_shader_explicit_vertex_parameter = 0, > >>> + SPV_AMD_shader_trinary_minmax, > >>> + SPV_AMD_gcn_shader, > >>> + SPV_KHR_shader_ballot, > >>> + SPV_AMD_shader_ballot, > >>> + SPV_AMD_gpu_shader_half_float, > >>> + SPV_KHR_shader_draw_parameters, > >>> + SPV_KHR_subgroup_vote, > >>> + SPV_KHR_16bit_storage, > >>> + SPV_KHR_device_group, > >>> + SPV_KHR_multiview, > >>> + SPV_NVX_multiview_per_view_attributes, > >>> + SPV_NV_viewport_array2, > >>> + SPV_NV_stereo_view_rendering, > >>> + SPV_NV_sample_mask_override_coverage, > >>> + SPV_NV_geometry_shader_passthrough, > >>> + SPV_AMD_texture_gather_bias_lod, > >>> + SPV_KHR_storage_buffer_storage_class, > >>> + SPV_KHR_variable_pointers, > >>> + SPV_AMD_gpu_shader_int16, > >>> + SPV_KHR_post_depth_coverage, > >>> + SPV_KHR_shader_atomic_counter_ops, > >>> + SPV_EXT_shader_stencil_export, > >>> + SPV_EXT_shader_viewport_index_layer, > >>> + SPV_AMD_shader_image_load_store_lod, > >>> + SPV_AMD_shader_fragment_mask, > >>> + SPV_EXTENSIONS_COUNT > >>> +} SpvExtension; > >>> + > >>> +const char *spirv_extensions_to_string(SpvExtension ext); > >>> + > >>> +#ifdef __cplusplus > >>> +} > >>> +#endif > >>> + > >>> +#endif /* SPIRV_EXTENSIONS */ > >>> > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev