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

Author: Rob Clark <robcl...@freedesktop.org>
Date:   Sun Jan  4 12:41:02 2015 -0500

freedreno/ir3: fix pos_regid > max_reg

We can't (or don't know how to) turn this off.  But it can end up being
stored to a higher reg # than what the shader uses, leading to
corruption.

Also we currently aren't clever enough to turn off frag_coord/frag_face
if the input is dead-code, so just fixup max_reg/max_half_reg.  Re-org
this a bit so both vp and fp reg footprint fixup are called by a common
fxn used also by ir3_cmdline.  Also add a few more output lines for
ir3_cmdline to make it easier to see what is going on.

Signed-off-by: Rob Clark <robcl...@freedesktop.org>

---

 src/gallium/drivers/freedreno/ir3/ir3_cmdline.c |   51 +++++++++--
 src/gallium/drivers/freedreno/ir3/ir3_ra.c      |    2 +-
 src/gallium/drivers/freedreno/ir3/ir3_shader.c  |  108 +++++++++++++++--------
 src/gallium/drivers/freedreno/ir3/ir3_shader.h  |    1 +
 4 files changed, 121 insertions(+), 41 deletions(-)

diff --git a/src/gallium/drivers/freedreno/ir3/ir3_cmdline.c 
b/src/gallium/drivers/freedreno/ir3/ir3_cmdline.c
index 6c334d2..4b60802 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3_cmdline.c
+++ b/src/gallium/drivers/freedreno/ir3/ir3_cmdline.c
@@ -42,15 +42,28 @@
 #include "instr-a3xx.h"
 #include "ir3.h"
 
+static void dump_reg(const char *name, uint32_t r)
+{
+       if (r != regid(63,0))
+               debug_printf("; %s: r%d.%c\n", name, r >> 2, "xyzw"[r & 0x3]);
+}
+
+static void dump_semantic(struct ir3_shader_variant *so,
+               unsigned sem, const char *name)
+{
+       uint32_t regid;
+       regid = ir3_find_output_regid(so, ir3_semantic_name(sem, 0));
+       dump_reg(name, regid);
+}
+
 static void dump_info(struct ir3_shader_variant *so, const char *str)
 {
-       struct ir3_info info;
        uint32_t *bin;
        const char *type = (so->type == SHADER_VERTEX) ? "VERT" : "FRAG";
 
        // for debug, dump some before/after info:
        // TODO make gpu_id configurable on cmdline
-       bin = ir3_assemble(so->ir, &info, 320);
+       bin = ir3_shader_assemble(so, 320);
        if (fd_mesa_debug & FD_DBG_DISASM) {
                struct ir3_block *block = so->ir->block;
                struct ir3_register *reg;
@@ -109,7 +122,7 @@ if (block) {
                }
 }
 
-               disasm_a3xx(bin, info.sizedwords, 0, so->type);
+               disasm_a3xx(bin, so->info.sizedwords, 0, so->type);
 
                debug_printf("; %s: outputs:", type);
                for (i = 0; i < so->outputs_count; i++) {
@@ -133,9 +146,37 @@ if (block) {
                }
                debug_printf("\n");
        }
-       debug_printf("; %s: %u instructions, %d half, %d full\n\n",
-                       type, info.instrs_count, info.max_half_reg + 1, 
info.max_reg + 1);
+
+       /* print generic shader info: */
+       debug_printf("; %s: %u instructions, %d half, %d full\n", type,
+                       so->info.instrs_count,
+                       so->info.max_half_reg + 1,
+                       so->info.max_reg + 1);
+
+       /* print shader type specific info: */
+       switch (so->type) {
+       case SHADER_VERTEX:
+               dump_semantic(so, TGSI_SEMANTIC_POSITION, "pos");
+               dump_semantic(so, TGSI_SEMANTIC_PSIZE, "psize");
+               break;
+       case SHADER_FRAGMENT:
+               dump_reg("pos (bary)", so->pos_regid);
+               dump_semantic(so, TGSI_SEMANTIC_POSITION, "posz");
+               dump_semantic(so, TGSI_SEMANTIC_COLOR, "color");
+               /* these two are hard-coded since we don't know how to
+                * program them to anything but all 0's...
+                */
+               if (so->frag_coord)
+                       debug_printf("; fragcoord: r0.x\n");
+               if (so->frag_face)
+                       debug_printf("; fragface: hr0.x\n");
+               break;
+       case SHADER_COMPUTE:
+               break;
+       }
        free(bin);
+
+       debug_printf("\n");
 }
 
 
diff --git a/src/gallium/drivers/freedreno/ir3/ir3_ra.c 
b/src/gallium/drivers/freedreno/ir3/ir3_ra.c
index eaeba0a..75ea4d6 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3_ra.c
+++ b/src/gallium/drivers/freedreno/ir3/ir3_ra.c
@@ -519,7 +519,7 @@ static int block_ra(struct ir3_ra_ctx *ctx, struct 
ir3_block *block)
         */
        if ((ctx->type == SHADER_FRAGMENT) && !block->parent) {
                unsigned i = 0, j;
-               if (ctx->frag_face) {
+               if (ctx->frag_face && (i < block->ninputs) && block->inputs[i]) 
{
                        /* if we have frag_face, it gets hr0.x */
                        instr_assign(ctx, block->inputs[i], REG_HALF | 0);
                        i += 4;
diff --git a/src/gallium/drivers/freedreno/ir3/ir3_shader.c 
b/src/gallium/drivers/freedreno/ir3/ir3_shader.c
index c21d0a2..5e43e28 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3_shader.c
+++ b/src/gallium/drivers/freedreno/ir3/ir3_shader.c
@@ -50,23 +50,61 @@ delete_variant(struct ir3_shader_variant *v)
        free(v);
 }
 
+/* for vertex shader, the inputs are loaded into registers before the shader
+ * is executed, so max_regs from the shader instructions might not properly
+ * reflect the # of registers actually used, especially in case passthrough
+ * varyings.
+ *
+ * Likewise, for fragment shader, we can have some regs which are passed
+ * input values but never touched by the resulting shader (ie. as result
+ * of dead code elimination or simply because we don't know how to turn
+ * the reg off.
+ */
 static void
-assemble_variant(struct ir3_shader_variant *v)
+fixup_regfootprint(struct ir3_shader_variant *v)
 {
-       struct fd_context *ctx = fd_context(v->shader->pctx);
-       uint32_t gpu_id = ir3_shader_gpuid(v->shader);
-       uint32_t sz, *bin;
-
-       bin = ir3_assemble(v->ir, &v->info, gpu_id);
-       sz = v->info.sizedwords * 4;
-
-       v->bo = fd_bo_new(ctx->dev, sz,
-                       DRM_FREEDRENO_GEM_CACHE_WCOMBINE |
-                       DRM_FREEDRENO_GEM_TYPE_KMEM);
+       if (v->type == SHADER_VERTEX) {
+               unsigned i;
+               for (i = 0; i < v->inputs_count; i++) {
+                       /* skip frag inputs fetch via bary.f since their reg's 
are
+                        * not written by gpu before shader starts (and in fact 
the
+                        * regid's might not even be valid)
+                        */
+                       if (v->inputs[i].bary)
+                               continue;
+
+                       if (v->inputs[i].compmask) {
+                               int32_t regid = (v->inputs[i].regid + 3) >> 2;
+                               v->info.max_reg = MAX2(v->info.max_reg, regid);
+                       }
+               }
+               for (i = 0; i < v->outputs_count; i++) {
+                       int32_t regid = (v->outputs[i].regid + 3) >> 2;
+                       v->info.max_reg = MAX2(v->info.max_reg, regid);
+               }
+       } else if (v->type == SHADER_FRAGMENT) {
+               /* NOTE: not sure how to turn pos_regid off..  but this could
+                * be, for example, r1.x while max reg used by the shader is
+                * r0.*, in which case we need to fixup the reg footprint:
+                */
+               v->info.max_reg = MAX2(v->info.max_reg, v->pos_regid >> 2);
+               if (v->frag_coord)
+                       debug_assert(v->info.max_reg >= 0); /* hard coded r0.x 
*/
+               if (v->frag_face)
+                       debug_assert(v->info.max_half_reg >= 0); /* hr0.x */
+       }
+}
 
-       memcpy(fd_bo_map(v->bo), bin, sz);
+/* wrapper for ir3_assemble() which does some info fixup based on
+ * shader state.  Non-static since used by ir3_cmdline too.
+ */
+void * ir3_shader_assemble(struct ir3_shader_variant *v, uint32_t gpu_id)
+{
+       void *bin;
 
-       free(bin);
+       bin = ir3_assemble(v->ir, &v->info, gpu_id);
+       if (!bin)
+               return NULL;
 
        if (gpu_id >= 400) {
                v->instrlen = v->info.sizedwords / (2 * 16);
@@ -80,29 +118,32 @@ assemble_variant(struct ir3_shader_variant *v)
         */
        v->constlen = MAX2(v->constlen, v->info.max_const + 1);
 
-       /* no need to keep the ir around beyond this point: */
-       ir3_destroy(v->ir);
-       v->ir = NULL;
+       fixup_regfootprint(v);
+
+       return bin;
 }
 
-/* for vertex shader, the inputs are loaded into registers before the shader
- * is executed, so max_regs from the shader instructions might not properly
- * reflect the # of registers actually used:
- */
 static void
-fixup_vp_regfootprint(struct ir3_shader_variant *v)
+assemble_variant(struct ir3_shader_variant *v)
 {
-       unsigned i;
-       for (i = 0; i < v->inputs_count; i++) {
-               if (v->inputs[i].compmask) {
-                       int32_t regid = (v->inputs[i].regid + 3) >> 2;
-                       v->info.max_reg = MAX2(v->info.max_reg, regid);
-               }
-       }
-       for (i = 0; i < v->outputs_count; i++) {
-               int32_t regid = (v->outputs[i].regid + 3) >> 2;
-               v->info.max_reg = MAX2(v->info.max_reg, regid);
-       }
+       struct fd_context *ctx = fd_context(v->shader->pctx);
+       uint32_t gpu_id = ir3_shader_gpuid(v->shader);
+       uint32_t sz, *bin;
+
+       bin = ir3_shader_assemble(v, gpu_id);
+       sz = v->info.sizedwords * 4;
+
+       v->bo = fd_bo_new(ctx->dev, sz,
+                       DRM_FREEDRENO_GEM_CACHE_WCOMBINE |
+                       DRM_FREEDRENO_GEM_TYPE_KMEM);
+
+       memcpy(fd_bo_map(v->bo), bin, sz);
+
+       free(bin);
+
+       /* no need to keep the ir around beyond this point: */
+       ir3_destroy(v->ir);
+       v->ir = NULL;
 }
 
 /* reset before attempting to compile again.. */
@@ -162,9 +203,6 @@ create_variant(struct ir3_shader *shader, struct 
ir3_shader_key key)
                goto fail;
        }
 
-       if (shader->type == SHADER_VERTEX)
-               fixup_vp_regfootprint(v);
-
        if (fd_mesa_debug & FD_DBG_DISASM) {
                DBG("disassemble: type=%d, k={bp=%u,cts=%u,hp=%u}", v->type,
                        key.binning_pass, key.color_two_side, 
key.half_precision);
diff --git a/src/gallium/drivers/freedreno/ir3/ir3_shader.h 
b/src/gallium/drivers/freedreno/ir3/ir3_shader.h
index fcd5895..5207185 100644
--- a/src/gallium/drivers/freedreno/ir3/ir3_shader.h
+++ b/src/gallium/drivers/freedreno/ir3/ir3_shader.h
@@ -210,6 +210,7 @@ struct ir3_shader {
        uint32_t vpsrepl[8];
 };
 
+void * ir3_shader_assemble(struct ir3_shader_variant *v, uint32_t gpu_id);
 
 struct ir3_shader * ir3_shader_create(struct pipe_context *pctx,
                const struct tgsi_token *tokens, enum shader_t type);

_______________________________________________
mesa-commit mailing list
mesa-commit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-commit

Reply via email to