From 2f18698220d8b27991fab550c4721590d17278e0 Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Fri, 1 Jun 2012 13:16:58 -0700 Subject: i965/fs: Fix user-defined FS outputs with less than four components. OpenGL allows you to declare user-defined fragment shader outputs with less than four components: out ivec2 color; This makes sense if you're rendering to an RG format render target. Previously, we assumed that all color outputs had four components (like the built-in gl_FragColor/gl_FragData variables). This caused us to call emit_color_write for invalid indices, incrementing the output virtual GRF's reg_offset beyond the size of the register. This caused cascading failures: split_virtual_grfs would allocate new size-1 registers based on the virtual GRF size, but then proceed to rewrite the out-of-bounds accesses assuming that it had allocated enough new (contiguously numbered) registers. This resulted in instructions that accessed size-1 GRFs which register numbers beyond virtual_grf_next (i.e. registers that were never allocated). Finally, this manifested as live variable analysis and instruction scheduling accessing their temporary array with an out of bounds index (as they're all sized based on virtual_grf_next), and the program would segfault. It looks like the hardware's Render Target Write message requires you to send four components, even for RT formats such as RG or RGB. This patch continues to use all four MRFs, but doesn't bother to fill any data for the last few, which should be unused. +2 oglconforms. NOTE: This is a candidate for stable release branches. Signed-off-by: Kenneth Graunke Reviewed-by: Eric Anholt --- src/mesa/drivers/dri/i965/brw_fs.h | 1 + src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h index 9d1746cecc8..0c6c3e4e98a 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.h +++ b/src/mesa/drivers/dri/i965/brw_fs.h @@ -610,6 +610,7 @@ public: struct hash_table *variable_ht; ir_variable *frag_depth; fs_reg outputs[BRW_MAX_DRAW_BUFFERS]; + unsigned output_components[BRW_MAX_DRAW_BUFFERS]; fs_reg dual_src_output; int first_non_payload_grf; int max_grf; diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp index 845ab49972c..2634b0862a0 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp @@ -80,6 +80,7 @@ fs_visitor::visit(ir_variable *ir) /* Writing gl_FragColor outputs to all color regions. */ for (unsigned int i = 0; i < MAX2(c->key.nr_color_regions, 1); i++) { this->outputs[i] = *reg; + this->output_components[i] = 4; } } else if (ir->location == FRAG_RESULT_DEPTH) { this->frag_depth = ir; @@ -88,11 +89,16 @@ fs_visitor::visit(ir_variable *ir) assert(ir->location >= FRAG_RESULT_DATA0 && ir->location < FRAG_RESULT_DATA0 + BRW_MAX_DRAW_BUFFERS); + int vector_elements = + ir->type->is_array() ? ir->type->fields.array->vector_elements + : ir->type->vector_elements; + /* General color output. */ for (unsigned int i = 0; i < MAX2(1, ir->type->length); i++) { int output = ir->location - FRAG_RESULT_DATA0 + i; this->outputs[output] = *reg; - this->outputs[output].reg_offset += 4 * i; + this->outputs[output].reg_offset += vector_elements * i; + this->output_components[output] = vector_elements; } } } else if (ir->mode == ir_var_uniform) { @@ -2166,7 +2172,7 @@ fs_visitor::emit_fb_writes() this->current_annotation = ralloc_asprintf(this->mem_ctx, "FB write target %d", target); - for (int i = 0; i < 4; i++) + for (unsigned i = 0; i < this->output_components[target]; i++) emit_color_write(target, i, color_mrf); fs_inst *inst = emit(FS_OPCODE_FB_WRITE); -- cgit v1.2.3