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 <kenneth@whitecape.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
This commit is contained in:
Kenneth Graunke 2012-06-01 13:16:58 -07:00
parent cb18472eca
commit 2f18698220
2 changed files with 9 additions and 2 deletions

View file

@ -610,6 +610,7 @@ public:
struct hash_table *variable_ht; struct hash_table *variable_ht;
ir_variable *frag_depth; ir_variable *frag_depth;
fs_reg outputs[BRW_MAX_DRAW_BUFFERS]; fs_reg outputs[BRW_MAX_DRAW_BUFFERS];
unsigned output_components[BRW_MAX_DRAW_BUFFERS];
fs_reg dual_src_output; fs_reg dual_src_output;
int first_non_payload_grf; int first_non_payload_grf;
int max_grf; int max_grf;

View file

@ -80,6 +80,7 @@ fs_visitor::visit(ir_variable *ir)
/* Writing gl_FragColor outputs to all color regions. */ /* Writing gl_FragColor outputs to all color regions. */
for (unsigned int i = 0; i < MAX2(c->key.nr_color_regions, 1); i++) { for (unsigned int i = 0; i < MAX2(c->key.nr_color_regions, 1); i++) {
this->outputs[i] = *reg; this->outputs[i] = *reg;
this->output_components[i] = 4;
} }
} else if (ir->location == FRAG_RESULT_DEPTH) { } else if (ir->location == FRAG_RESULT_DEPTH) {
this->frag_depth = ir; this->frag_depth = ir;
@ -88,11 +89,16 @@ fs_visitor::visit(ir_variable *ir)
assert(ir->location >= FRAG_RESULT_DATA0 && assert(ir->location >= FRAG_RESULT_DATA0 &&
ir->location < FRAG_RESULT_DATA0 + BRW_MAX_DRAW_BUFFERS); 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. */ /* General color output. */
for (unsigned int i = 0; i < MAX2(1, ir->type->length); i++) { for (unsigned int i = 0; i < MAX2(1, ir->type->length); i++) {
int output = ir->location - FRAG_RESULT_DATA0 + i; int output = ir->location - FRAG_RESULT_DATA0 + i;
this->outputs[output] = *reg; 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) { } else if (ir->mode == ir_var_uniform) {
@ -2166,7 +2172,7 @@ fs_visitor::emit_fb_writes()
this->current_annotation = ralloc_asprintf(this->mem_ctx, this->current_annotation = ralloc_asprintf(this->mem_ctx,
"FB write target %d", "FB write target %d",
target); 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); emit_color_write(target, i, color_mrf);
fs_inst *inst = emit(FS_OPCODE_FB_WRITE); fs_inst *inst = emit(FS_OPCODE_FB_WRITE);