nir: Actually do load/store vectorization beyond vec2
authorKenneth Graunke <kenneth@whitecape.org>
Fri, 27 Mar 2020 09:05:56 +0000 (02:05 -0700)
committerKenneth Graunke <kenneth@whitecape.org>
Thu, 23 Apr 2020 04:22:36 +0000 (21:22 -0700)
nir_opt_load_store_vectorize has an is_strided_vector() function that
looks for types with weird explicit strides.  It does so by comparing
the explicit stride against the type-size-derived typical stride.

This had a subtle bug.  Simple vector types (vec2/3/4) have no explicit
stride, so glsl_get_explicit_stride() returns 0.  This never matches the
typical stride for a vector, so is_strided_vector() would return true
for basically any vector type, causing the vectorizer to bail.

I found this by looking at a compute shader with scalar SSBO loads at
offsets 0x220, 0x224, 0x228, 0x22c.  nir_opt_load_store_vectorize would
properly vectorize the first two into a vec2 load, but would refuse to
extend it to a vec3 and ultimately vec4 load because is_strided_vector()
saw a vec2 and freaked out.

Neither ACO nor ANV do load/store vectorization before lowering derefs,
so this shouldn't affect them.  However, I'd like to fix this bug to
avoid the trap for anyone who decides to in the future.  In a branch
where anv used this lowering, this cut an additional 38% of the send
messages in the shader by properly vectorizing more things.

Reviewed-by: Rhys Perry <pendingchaos02@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4255>

src/compiler/nir/nir_opt_load_store_vectorize.c

index cf8d0ef0ddd797271a947d5c53eeb480e6916577..c31c8d293bf76598133485d1d5ca0e122481b5b4 100644 (file)
@@ -1057,7 +1057,8 @@ static bool
 is_strided_vector(const struct glsl_type *type)
 {
    if (glsl_type_is_vector(type)) {
-      return glsl_get_explicit_stride(type) !=
+      unsigned explicit_stride = glsl_get_explicit_stride(type);
+      return explicit_stride != 0 && explicit_stride !=
              type_scalar_size_bytes(glsl_get_array_element(type));
    } else {
       return false;