[Mesa-dev] [PATCH v3 2/5] glsl: add support for the imageSize builtin
Francisco Jerez
currojerez at riseup.net
Wed Aug 19 06:36:28 PDT 2015
Martin Peres <martin.peres at linux.intel.com> writes:
> The code is heavily inspired from Francisco Jerez's code supporting the
> image_load_store extension.
>
> Backends willing to support this builtin should handle
> __intrinsic_image_size.
>
> v2: Based on the review of Ilia Mirkin
> - Enable the extension for GLES 3.1
> - Fix indentation
> - Fix the return type (float to int, number of components for CubeImages)
> - Add a warning related to GLES 3.1
>
> v3: Based on the review of Francisco Jerez
> - Refactor the code to share both add_image_function and _image with the other
> image-related functions
>
> Signed-off-by: Martin Peres <martin.peres at linux.intel.com>
> ---
> src/glsl/builtin_functions.cpp | 109 +++++++++++++++++++++++++++++++++++------
> 1 file changed, 93 insertions(+), 16 deletions(-)
>
> diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp
> index 2175c66..5d0a825 100644
> --- a/src/glsl/builtin_functions.cpp
> +++ b/src/glsl/builtin_functions.cpp
> @@ -399,6 +399,13 @@ shader_image_load_store(const _mesa_glsl_parse_state *state)
> }
>
> static bool
> +shader_image_size(const _mesa_glsl_parse_state *state)
> +{
> + return (state->is_version(430, 310) ||
> + state->ARB_shader_image_size_enable);
> +}
> +
> +static bool
> gs_streams(const _mesa_glsl_parse_state *state)
> {
> return gpu_shader5(state) && gs_only(state);
> @@ -492,6 +499,11 @@ private:
> /** Create a new function and add the given signatures. */
> void add_function(const char *name, ...);
>
> + typedef ir_function_signature *(builtin_builder::*image_prototype_ctr)(const glsl_type *image_type,
> + const char *intrinsic_name,
> + unsigned num_arguments,
> + unsigned flags);
> +
> enum image_function_flags {
> IMAGE_FUNCTION_EMIT_STUB = (1 << 0),
> IMAGE_FUNCTION_RETURNS_VOID = (1 << 1),
> @@ -507,6 +519,7 @@ private:
> */
> void add_image_function(const char *name,
> const char *intrinsic_name,
> + image_prototype_ctr prototype,
> unsigned num_arguments,
> unsigned flags);
>
> @@ -708,7 +721,12 @@ private:
> const char *intrinsic_name,
> unsigned num_arguments,
> unsigned flags);
> - ir_function_signature *_image(const glsl_type *image_type,
> + ir_function_signature *_image_size_prototype(const glsl_type *image_type,
> + const char *intrinsic_name,
> + unsigned num_arguments,
> + unsigned flags);
The indentation looks weird. Mixing tabs and spaces?
> + ir_function_signature *_image(image_prototype_ctr prototype,
> + const glsl_type *image_type,
> const char *intrinsic_name,
> unsigned num_arguments,
> unsigned flags);
> @@ -2552,6 +2570,7 @@ builtin_builder::add_function(const char *name, ...)
> void
> builtin_builder::add_image_function(const char *name,
> const char *intrinsic_name,
> + image_prototype_ctr prototype,
> unsigned num_arguments,
> unsigned flags)
> {
> @@ -2590,12 +2609,13 @@ builtin_builder::add_image_function(const char *name,
> glsl_type::uimage2DMS_type,
> glsl_type::uimage2DMSArray_type
> };
> +
> ir_function *f = new(mem_ctx) ir_function(name);
>
> for (unsigned i = 0; i < ARRAY_SIZE(types); ++i) {
> if (types[i]->sampler_type != GLSL_TYPE_FLOAT ||
> (flags & IMAGE_FUNCTION_SUPPORTS_FLOAT_DATA_TYPE))
> - f->add_signature(_image(types[i], intrinsic_name,
> + f->add_signature(_image(prototype, types[i], intrinsic_name,
> num_arguments, flags));
> }
>
> @@ -2608,43 +2628,57 @@ builtin_builder::add_image_functions(bool glsl)
> const unsigned flags = (glsl ? IMAGE_FUNCTION_EMIT_STUB : 0);
>
> add_image_function(glsl ? "imageLoad" : "__intrinsic_image_load",
> - "__intrinsic_image_load", 0,
> - (flags | IMAGE_FUNCTION_HAS_VECTOR_DATA_TYPE |
> + "__intrinsic_image_load",
> + &builtin_builder::_image_prototype, 0,
> + (flags | IMAGE_FUNCTION_HAS_VECTOR_DATA_TYPE |
> IMAGE_FUNCTION_SUPPORTS_FLOAT_DATA_TYPE |
> IMAGE_FUNCTION_READ_ONLY));
>
> add_image_function(glsl ? "imageStore" : "__intrinsic_image_store",
> - "__intrinsic_image_store", 1,
> + "__intrinsic_image_store",
> + &builtin_builder::_image_prototype, 1,
> (flags | IMAGE_FUNCTION_RETURNS_VOID |
> IMAGE_FUNCTION_HAS_VECTOR_DATA_TYPE |
> IMAGE_FUNCTION_SUPPORTS_FLOAT_DATA_TYPE |
> IMAGE_FUNCTION_WRITE_ONLY));
>
> add_image_function(glsl ? "imageAtomicAdd" : "__intrinsic_image_atomic_add",
> - "__intrinsic_image_atomic_add", 1, flags);
> + "__intrinsic_image_atomic_add",
> + &builtin_builder::_image_prototype, 1, flags);
>
> add_image_function(glsl ? "imageAtomicMin" : "__intrinsic_image_atomic_min",
> - "__intrinsic_image_atomic_min", 1, flags);
> + "__intrinsic_image_atomic_min",
> + &builtin_builder::_image_prototype, 1, flags);
>
> add_image_function(glsl ? "imageAtomicMax" : "__intrinsic_image_atomic_max",
> - "__intrinsic_image_atomic_max", 1, flags);
> + "__intrinsic_image_atomic_max",
> + &builtin_builder::_image_prototype, 1, flags);
>
> add_image_function(glsl ? "imageAtomicAnd" : "__intrinsic_image_atomic_and",
> - "__intrinsic_image_atomic_and", 1, flags);
> + "__intrinsic_image_atomic_and",
> + &builtin_builder::_image_prototype, 1, flags);
>
> add_image_function(glsl ? "imageAtomicOr" : "__intrinsic_image_atomic_or",
> - "__intrinsic_image_atomic_or", 1, flags);
> + "__intrinsic_image_atomic_or",
> + &builtin_builder::_image_prototype, 1, flags);
>
> add_image_function(glsl ? "imageAtomicXor" : "__intrinsic_image_atomic_xor",
> - "__intrinsic_image_atomic_xor", 1, flags);
> + "__intrinsic_image_atomic_xor",
> + &builtin_builder::_image_prototype, 1, flags);
>
> add_image_function((glsl ? "imageAtomicExchange" :
> "__intrinsic_image_atomic_exchange"),
> - "__intrinsic_image_atomic_exchange", 1, flags);
> + "__intrinsic_image_atomic_exchange",
> + &builtin_builder::_image_prototype, 1, flags);
>
> add_image_function((glsl ? "imageAtomicCompSwap" :
> "__intrinsic_image_atomic_comp_swap"),
> - "__intrinsic_image_atomic_comp_swap", 2, flags);
> + "__intrinsic_image_atomic_comp_swap",
> + &builtin_builder::_image_prototype, 2, flags);
> +
> + add_image_function(glsl ? "imageSize" : "__intrinsic_image_size",
> + "__intrinsic_image_size",
> + &builtin_builder::_image_size_prototype, 1, flags);
> }
>
> ir_variable *
> @@ -4818,13 +4852,56 @@ builtin_builder::_image_prototype(const glsl_type *image_type,
> }
>
> ir_function_signature *
> -builtin_builder::_image(const glsl_type *image_type,
> +builtin_builder::_image_size_prototype(const glsl_type *image_type,
> + const char *intrinsic_name,
> + unsigned num_arguments,
> + unsigned flags)
> +{
> + const glsl_type *ret_type;
> + unsigned num_components = image_type->coordinate_components();
> +
> + /* From the ARB_shader_image_size extension:
> + * "Cube images return the dimensions of one face."
> + */
> + if (image_type == glsl_type::imageCube_type ||
> + image_type == glsl_type::iimageCube_type ||
> + image_type == glsl_type::uimageCube_type) {
If you like you could do (image_type->sampler_dimensionality ==
GLSL_SAMPLER_DIM_CUBE && !image_type->sampler_array) here to avoid
enumerating all image data types.
> + num_components = 2;
> + }
> +
> + /* FIXME: Add the highp precision qualified for GLES 3.10 when it is
> + * becomes supported by mesa.
> + */
s/qualified/qualifier/ s/becomes //?
> + ret_type = glsl_type::get_instance(GLSL_TYPE_INT, num_components, 1);
> +
> + ir_variable *image = in_var(image_type, "image");
> + ir_function_signature *sig = new_sig(ret_type, shader_image_size, 1, image);
> +
> + /* Set the maximal set of qualifiers allowed for this image
> + * built-in. Function calls with arguments having fewer
> + * qualifiers than present in the prototype are allowed by the
> + * spec, but not with more, i.e. this will make the compiler
> + * accept everything that needs to be accepted, and reject cases
> + * like loads from write-only or stores to read-only images.
> + */
> + image->data.image_read_only = true;
> + image->data.image_write_only = true;
> + image->data.image_coherent = true;
> + image->data.image_volatile = true;
> + image->data.image_restrict = true;
> +
> + return sig;
> +}
> +
> +ir_function_signature *
> +builtin_builder::_image(image_prototype_ctr prototype,
> + const glsl_type *image_type,
> const char *intrinsic_name,
> unsigned num_arguments,
> unsigned flags)
> {
> - ir_function_signature *sig = _image_prototype(image_type, intrinsic_name,
> - num_arguments, flags);
> + ir_function_signature *sig = (this->*prototype)(image_type, intrinsic_name,
> + num_arguments, flags);
Weird indentation? With my nit-picks fixed this patch is:
Reviewed-by: Francisco Jerez <currojerez at riseup.net>
>
> if (flags & IMAGE_FUNCTION_EMIT_STUB) {
> ir_factory body(&sig->body, mem_ctx);
> --
> 2.5.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150819/4ed9644e/attachment.sig>
More information about the mesa-dev
mailing list