[Mesa-dev] [PATCH] [RFC] r600g: do not pick TGSI input registers at random

[Constantine Kharlamov]

tgsi_declaration() configures inputs. Then tgsi_interp_egcm() uses one
of them for interpolation. Unfortunately it was choosing registers by using
Src[0].Register.Index of the interpolation instruction as an index into shader
inputs. Of course it was working by pure coincidence. E.g. for pidglit test
"interpolateAtSample" the order of inputs happened to be IMM[1], then IN[0],
then IN[1]. So instead of indexing into IN[1] it was indexing into IN[0].

The workaround is saving tgsi_file_type in inputs at tgsi_declaration(), and
later at tgsi_interp_egcm() (possibly in other places too) cycling through the
inputs searching for the appropriate one.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=100785

------

Because of unfamilarity with the code architecture I am unsure how to handle
some things, i.e.:

α) For some reason tgsi_declaration() never sees "ctx->shader->input[0]" (i.e.
IMM[1]). It's configured at the end of "allocate_system_value_inputs()" which
is fine, but I can't snoop around for places other than tgsi_declaration()
where inputs could be configured — it would be unreliable and fragile. I tried
forcing to start parse from the beginning just before the cycle where
tgsi_declaration() is called, but it didn't help, for some reason the "0" input
does not appear in the cycle.

I thought at this point it would be better to just ask.

β) I put an assert at tgsi_interp_egcm() in case some bug left the input
register unconfigured. In release version it'd return -ECANCELED — the other
possible fail I found in the function is -ENOMEM, so I don't know if there's a
better value. That said, I don't think it matters much either — there's a
unique print+assert.

Signed-off-by: Constantine Kharlamov <Hi-Angel at yandex.ru>
---
 src/gallium/drivers/r600/r600_shader.c | 23 +++++++++++++++++++++--
 src/gallium/drivers/r600/r600_shader.h |  5 +++--
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c
index 156dba085d..b373a70bca 100644
--- a/src/gallium/drivers/r600/r600_shader.c
+++ b/src/gallium/drivers/r600/r600_shader.c
@@ -861,6 +861,7 @@ static int tgsi_declaration(struct r600_shader_ctx *ctx)
 			ctx->shader->input[i].interpolate = d->Interp.Interpolate;
 			ctx->shader->input[i].interpolate_location = d->Interp.Location;
 			ctx->shader->input[i].gpr = ctx->file_offset[TGSI_FILE_INPUT] + d->Range.First + j;
+			ctx->shader->input[i].type = TGSI_FILE_INPUT;
 			if (ctx->type == PIPE_SHADER_FRAGMENT) {
 				ctx->shader->input[i].spi_sid = r600_spi_sid(&ctx->shader->input[i]);
 				switch (ctx->shader->input[i].name) {
@@ -905,6 +906,7 @@ static int tgsi_declaration(struct r600_shader_ctx *ctx)
 			ctx->shader->output[i].gpr = ctx->file_offset[TGSI_FILE_OUTPUT] + d->Range.First + j;
 			ctx->shader->output[i].interpolate = d->Interp.Interpolate;
 			ctx->shader->output[i].write_mask = d->Declaration.UsageMask;
+			ctx->shader->input[i].type = TGSI_FILE_OUTPUT;
 			if (ctx->type == PIPE_SHADER_VERTEX ||
 			    ctx->type == PIPE_SHADER_GEOMETRY ||
 			    ctx->type == PIPE_SHADER_TESS_EVAL) {
@@ -6316,17 +6318,34 @@ static int tgsi_msb(struct r600_shader_ctx *ctx)
 	return 0;
 }
 
+static int tgsi_index_reg_in_inputs(const struct r600_shader *in, const struct tgsi_src_register *reg)
+{
+	for (int i = 0, same_type_i = 0; i < in->ninput; ++i) {
+		if (in->input[i].type == reg->File) {
+			if (same_type_i == reg->Index)
+				return i;
+			else
+				++same_type_i;
+		}
+	}
+	return -1;
+}
+
 static int tgsi_interp_egcm(struct r600_shader_ctx *ctx)
 {
 	struct tgsi_full_instruction *inst = &ctx->parse.FullToken.FullInstruction;
 	struct r600_bytecode_alu alu;
 	int r, i = 0, k, interp_gpr, interp_base_chan, tmp, lasti;
 	unsigned location;
-	int input;
+	const int input = tgsi_index_reg_in_inputs(ctx->shader, &inst->Src[0].Register);
 
 	assert(inst->Src[0].Register.File == TGSI_FILE_INPUT);
 
-	input = inst->Src[0].Register.Index;
+	if (input == -1) {
+		R600_ERR("The register not found in inputs!");
+		assert(false);
+		return -ECANCELED;
+	}
 
 	/* Interpolators have been marked for use already by allocate_system_value_inputs */
 	if (inst->Instruction.Opcode == TGSI_OPCODE_INTERP_OFFSET ||
diff --git a/src/gallium/drivers/r600/r600_shader.h b/src/gallium/drivers/r600/r600_shader.h
index cfdb020033..ea141e43b5 100644
--- a/src/gallium/drivers/r600/r600_shader.h
+++ b/src/gallium/drivers/r600/r600_shader.h
@@ -45,15 +45,16 @@ struct r600_shader_io {
 	unsigned		name;
 	unsigned		gpr;
 	unsigned		done;
+	enum tgsi_file_type	type;
 	int			sid;
 	int			spi_sid;
 	unsigned		interpolate;
 	unsigned		ij_index;
-	unsigned        interpolate_location; //  TGSI_INTERPOLATE_LOC_CENTER, CENTROID, SAMPLE
+	unsigned		interpolate_location; //  TGSI_INTERPOLATE_LOC_CENTER, CENTROID, SAMPLE
 	unsigned		lds_pos; /* for evergreen */
 	unsigned		back_color_input;
 	unsigned		write_mask;
-	int				ring_offset;
+	int			ring_offset;
 };
 
 struct r600_shader {
-- 
2.13.0