[Freedreno] [PATCH] ir3_compiler/nir: fix imageSize() for buffer-backed images

Eduardo Lima Mitev elima at igalia.com
Tue Oct 23 19:03:27 UTC 2018


GL_EXT_texture_buffer introduced texture buffers, which can be used
in shaders through a new type imageBuffer.

Because how image access is implemented in freedreno, calling
imageSize on an imageBuffer returns the size in bytes instead of texels,
which is incorrect.

This patch adds a division of imageSize result by the bytes-per-pixel
of the image format, when image is buffer-backed.

Fixes all tests under
dEQP-GLES31.functional.image_load_store.buffer.image_size.*

v2: Pre-compute and submit the log2 of the image format's bpp as shader
    constant instead of emitting the LOG2 instruction in code. (Rob Clark)
---
 .../drivers/freedreno/ir3/ir3_compiler_nir.c  | 23 +++++++++++++++++++
 .../drivers/freedreno/ir3/ir3_shader.c        | 10 ++++++++
 2 files changed, 33 insertions(+)

diff --git a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
index 197196383b0..7a3c8a8579c 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
+++ b/src/gallium/drivers/freedreno/ir3/ir3_compiler_nir.c
@@ -2035,6 +2035,29 @@ emit_intrinsic_image_size(struct ir3_context *ctx, nir_intrinsic_instr *intr,
 
 	split_dest(b, tmp, sam, 0, 4);
 
+	/* get_size instruction returns size in bytes instead of texels
+	 * for imageBuffer, so we need to divide it by the pixel size
+	 * of the image format.
+	 *
+	 * TODO: This is at least true on a5xx. Check other gens.
+	 */
+	enum glsl_sampler_dim dim =
+		glsl_get_sampler_dim(glsl_without_array(var->type));
+	if (dim == GLSL_SAMPLER_DIM_BUF) {
+		/* Since all the possible values the divisor can take are
+		 * power-of-two (4, 8, or 16), the division is implemented
+		 * as a shift-right.
+		 * During shader setup, the log2 of the image format's
+		 * bytes-per-pixel should have been emitted in 2nd slot of
+		 * image_dims. See ir3_shader::emit_image_dims().
+		 */
+		unsigned cb = regid(ctx->so->constbase.image_dims, 0) +
+			ctx->so->const_layout.image_dims.off[var->data.driver_location];
+		struct ir3_instruction *aux = create_uniform(ctx, cb + 1);
+
+		tmp[0] = ir3_SHR_B(b, tmp[0], 0, aux, 0);
+	}
+
 	for (unsigned i = 0; i < ncoords; i++)
 		dst[i] = tmp[i];
 
diff --git a/src/gallium/drivers/freedreno/ir3/ir3_shader.c b/src/gallium/drivers/freedreno/ir3/ir3_shader.c
index 9bf0a7f999c..de59e5888c6 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3_shader.c
+++ b/src/gallium/drivers/freedreno/ir3/ir3_shader.c
@@ -699,6 +699,16 @@ emit_image_dims(struct fd_context *ctx, const struct ir3_shader_variant *v,
 				} else {
 					dims[off + 2] = rsc->slices[lvl].size0;
 				}
+			} else {
+				/* For buffer-backed images, the log2 of the format's
+				 * bytes-per-pixel is placed on the 2nd slot. This is useful
+				 * when emitting image_size instructions, for which we need
+				 * to divide by bpp for image buffers. Since the bpp
+				 * can only be power-of-two, the division is implemented
+				 * as a SHR, and for that it is handy to have the log2 of
+				 * bpp as a constant.
+				 */
+				dims[off + 1] = log (dims[off + 0]) / log (2);
 			}
 		}
 
-- 
2.19.1



More information about the Freedreno mailing list