Mesa (main): glsl: Prohibit implicit conversion of mem parameter in atomicOP functions

[GitLab Mirror]

Module: Mesa
Branch: main
Commit: c0f623e62fb02faf24dc43179222b289a0e03683
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=c0f623e62fb02faf24dc43179222b289a0e03683

Author: Danylo Piliaiev <danylo.piliaiev at globallogic.com>
Date:   Wed Apr 29 14:08:48 2020 +0300

glsl: Prohibit implicit conversion of mem parameter in atomicOP functions

Per OpenGL Shading Language, section 8.11. "Atomic Memory Functions"
first argument "mem" of all atomicOP functions is inout.
The same is true for ARB_shader_storage_buffer_object and
GL_INTEL_shader_atomic_float_minmax

For implicit conversion of inout parameters it is required for type
to support bi-directional conversion, since there is no such types
in glsl - implicit conversion is effectively prohibited.

Alternatively we could have marked atomic_var parameter of built-in
atomicOP functions as inout, however it opens another can of worms
during NIR lowerings.

Fixes: ea0a1f5beb22982a886ba862ba95f92c9e35165a
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/2837
Signed-off-by: Danylo Piliaiev <danylo.piliaiev at globallogic.com>
Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4887>

---

 src/compiler/glsl/builtin_functions.cpp                 | 4 ++++
 src/compiler/glsl/ir.cpp                                | 1 +
 src/compiler/glsl/ir.h                                  | 6 ++++++
 src/compiler/glsl/ir_function.cpp                       | 5 +++--
 src/gallium/drivers/llvmpipe/ci/llvmpipe-glslparser.txt | 4 ----
 5 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/compiler/glsl/builtin_functions.cpp b/src/compiler/glsl/builtin_functions.cpp
index 9f3b526671e..601263e4238 100644
--- a/src/compiler/glsl/builtin_functions.cpp
+++ b/src/compiler/glsl/builtin_functions.cpp
@@ -7340,6 +7340,8 @@ builtin_builder::_atomic_op2(const char *intrinsic,
    ir_variable *data = in_var(type, "atomic_data");
    MAKE_SIG(type, avail, 2, atomic, data);
 
+   atomic->data.implicit_conversion_prohibited = true;
+
    ir_variable *retval = body.make_temp(type, "atomic_retval");
    body.emit(call(shader->symbols->get_function(intrinsic), retval,
                   sig->parameters));
@@ -7357,6 +7359,8 @@ builtin_builder::_atomic_op3(const char *intrinsic,
    ir_variable *data2 = in_var(type, "atomic_data2");
    MAKE_SIG(type, avail, 3, atomic, data1, data2);
 
+   atomic->data.implicit_conversion_prohibited = true;
+
    ir_variable *retval = body.make_temp(type, "atomic_retval");
    body.emit(call(shader->symbols->get_function(intrinsic), retval,
                   sig->parameters));
diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
index b2158e0888a..97b302a744a 100644
--- a/src/compiler/glsl/ir.cpp
+++ b/src/compiler/glsl/ir.cpp
@@ -2073,6 +2073,7 @@ ir_variable::ir_variable(const struct glsl_type *type, const char *name,
    this->data.stream = 0;
    this->data.xfb_buffer = -1;
    this->data.xfb_stride = -1;
+   this->data.implicit_conversion_prohibited = false;
 
    this->interface_type = NULL;
 
diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
index aa7c5d6124c..a22c27348d2 100644
--- a/src/compiler/glsl/ir.h
+++ b/src/compiler/glsl/ir.h
@@ -892,6 +892,12 @@ public:
        */
       unsigned bound:1;
 
+      /**
+       * Non-zero if the variable shall not be implicitly converted during
+       * functions matching.
+       */
+      unsigned implicit_conversion_prohibited:1;
+
       /**
        * Emit a warning if this variable is accessed.
        */
diff --git a/src/compiler/glsl/ir_function.cpp b/src/compiler/glsl/ir_function.cpp
index a38e5285292..3d2374fdd96 100644
--- a/src/compiler/glsl/ir_function.cpp
+++ b/src/compiler/glsl/ir_function.cpp
@@ -83,8 +83,9 @@ parameter_lists_match(_mesa_glsl_parse_state *state,
 
       case ir_var_const_in:
       case ir_var_function_in:
-	 if (!actual->type->can_implicitly_convert_to(param->type, state))
-	    return PARAMETER_LIST_NO_MATCH;
+         if (param->data.implicit_conversion_prohibited ||
+             !actual->type->can_implicitly_convert_to(param->type, state))
+            return PARAMETER_LIST_NO_MATCH;
 	 break;
 
       case ir_var_function_out:
diff --git a/src/gallium/drivers/llvmpipe/ci/llvmpipe-glslparser.txt b/src/gallium/drivers/llvmpipe/ci/llvmpipe-glslparser.txt
index 2222fd42316..bab460ac4fa 100644
--- a/src/gallium/drivers/llvmpipe/ci/llvmpipe-glslparser.txt
+++ b/src/gallium/drivers/llvmpipe/ci/llvmpipe-glslparser.txt
@@ -704,10 +704,6 @@ spec/arb_shader_stencil_export/preprocessor/disabled-undefined-core.geom: skip
 spec/arb_shader_stencil_export/preprocessor/disabled-undefined-core.tesc: skip
 spec/arb_shader_stencil_export/preprocessor/disabled-undefined-core.tese: skip
 spec/arb_shader_stencil_export/preprocessor/disabled-undefined-core.vert: skip
-spec/arb_shader_storage_buffer_object/compiler/atomiccompswap-implicit-conversion.vert: crash
-spec/arb_shader_storage_buffer_object/compiler/atomicmin-array-element-implicit-conversion.vert: crash
-spec/arb_shader_storage_buffer_object/compiler/atomicmin-implicit-conversion.vert: crash
-spec/arb_shader_storage_buffer_object/compiler/atomicmin-swizzle-implicit-conversion.vert: crash
 spec/arb_shader_storage_buffer_object/preprocessor/disabled-defined-compat.frag: skip
 spec/arb_shader_storage_buffer_object/preprocessor/disabled-defined-compat.vert: skip
 spec/arb_shader_storage_buffer_object/preprocessor/disabled-defined-core.comp: skip