[Mesa-dev] [PATCH v2 23/82] glsl: Do not do CSE for expressions involving SSBO loads

[Jordan Justen]

I wanted to question whether this was required, based on this text
from the extension spec:

"The ability to write to buffer objects creates the potential for
 multiple independent shader invocations to read and write the same
 underlying memory. The same issue exists with the
 ARB_shader_image_load_store extension provided in OpenGL 4.2, which
 can write to texture objects and buffers. In both cases, the
 specification makes few guarantees related to the relative order of
 memory reads and writes performed by the shader invocations."

But I'm not sure if we can reconcile CSE with 'memoryBarrier' and
'barrier'. curro, any thoughts from image load/store?

-Jordan

On 2015-06-03 00:01:13, Iago Toral Quiroga wrote:
> SSBOs are read/write and this CSE pass only handles read-only variables.
> ---
>  src/glsl/opt_cse.cpp | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/src/glsl/opt_cse.cpp b/src/glsl/opt_cse.cpp
> index 4b8e9a0..a05ab46 100644
> --- a/src/glsl/opt_cse.cpp
> +++ b/src/glsl/opt_cse.cpp
> @@ -245,6 +245,28 @@ contains_rvalue(ir_rvalue *haystack, ir_rvalue *needle)
>  }
>  
>  static bool
> +expression_contains_ssbo_load(ir_expression *expr)
> +{
> +   if (expr->operation == ir_binop_ssbo_load)
> +      return true;
> +
> +   for (unsigned i = 0; i < expr->get_num_operands(); i++) {
> +      ir_rvalue *op = expr->operands[i];
> +      if (op->ir_type == ir_type_expression &&
> +          expression_contains_ssbo_load(op->as_expression())) {
> +         return true;
> +      } else if (op->ir_type == ir_type_swizzle) {
> +         ir_swizzle *swizzle = op->as_swizzle();
> +         ir_expression *val = swizzle->val->as_expression();
> +         if (val && expression_contains_ssbo_load(val))
> +            return true;
> +      }
> +   }
> +
> +   return false;
> +}
> +
> +static bool
>  is_cse_candidate(ir_rvalue *ir)
>  {
>     /* Our temporary variable assignment generation isn't ready to handle
> @@ -260,7 +282,16 @@ is_cse_candidate(ir_rvalue *ir)
>      * to variable-index array dereferences at some point.
>      */
>     switch (ir->ir_type) {
> -   case ir_type_expression:
> +   case ir_type_expression: {
> +         /* Skip expressions involving SSBO loads, since these operate on
> +          * read-write variables, meaning that the same ssbo_load expression
> +          * may return a different value if the underlying buffer storage
> +          * is written in between.
> +          */
> +         if (expression_contains_ssbo_load(ir->as_expression()))
> +            return false;
> +      }
> +      break;
>     case ir_type_texture:
>        break;
>     default:
> -- 
> 1.9.1
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev