[Mesa-dev] [PATCH 06/10] i965/fs: Add support for user-defined out variables.
Ian Romanick
idr at freedesktop.org
Tue Nov 8 14:50:30 PST 2011
On 11/04/2011 03:01 PM, Eric Anholt wrote:
> Before, I was tracking the ir_variable * found for gl_FragColor or
> gl_FragData[]. Instead, when visiting those variables, set up an
> array of per-render-target fs_regs to copy the output data from. This
> cleans up the color emit path, while making handling of multiple
> user-defined out variables easier.
>
> Note that brw_type_for_base_type() now returns a sensible value for
> arrays. This probably obsoletes a bunch of type overrides we have
> laying around.
> ---
> src/mesa/drivers/dri/i965/brw_fs.h | 8 +-
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 80 ++++++++++++++-----------
> src/mesa/drivers/dri/i965/brw_shader.cpp | 1 +
> 3 files changed, 50 insertions(+), 39 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index e2ad649..854a935 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -376,9 +376,8 @@ public:
> else
> this->reg_null_cmp = reg_null_f;
>
> - this->frag_color = NULL;
> - this->frag_data = NULL;
> this->frag_depth = NULL;
> + memset(this->outputs, 0, sizeof(this->outputs));
> this->first_non_payload_grf = 0;
>
> this->current_annotation = NULL;
> @@ -526,7 +525,7 @@ public:
> void emit_if_gen6(ir_if *ir);
> void emit_unspill(fs_inst *inst, fs_reg reg, uint32_t spill_offset);
>
> - void emit_color_write(int index, int first_color_mrf, fs_reg color);
> + void emit_color_write(int target, int index, int first_color_mrf);
> void emit_fb_writes();
> bool try_rewrite_rhs_to_dst(ir_assignment *ir,
> fs_reg dst,
> @@ -574,7 +573,8 @@ public:
> int *params_remap;
>
> struct hash_table *variable_ht;
> - ir_variable *frag_color, *frag_data, *frag_depth;
> + ir_variable *frag_depth;
> + fs_reg outputs[BRW_MAX_DRAW_BUFFERS];
> int first_non_payload_grf;
> int urb_setup[FRAG_ATTRIB_MAX];
> bool kill_emitted;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 15009dc..e28a2e4 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -58,14 +58,6 @@ fs_visitor::visit(ir_variable *ir)
> if (variable_storage(ir))
> return;
>
> - if (strcmp(ir->name, "gl_FragColor") == 0) {
> - this->frag_color = ir;
> - } else if (strcmp(ir->name, "gl_FragData") == 0) {
> - this->frag_data = ir;
> - } else if (strcmp(ir->name, "gl_FragDepth") == 0) {
> - this->frag_depth = ir;
> - }
> -
> if (ir->mode == ir_var_in) {
> if (!strcmp(ir->name, "gl_FragCoord")) {
> reg = emit_fragcoord_interpolation(ir);
> @@ -77,9 +69,32 @@ fs_visitor::visit(ir_variable *ir)
> assert(reg);
> hash_table_insert(this->variable_ht, reg, ir);
> return;
> - }
> + } else if (ir->mode == ir_var_out) {
> + reg = new(this->mem_ctx) fs_reg(this, ir->type);
> +
> + if (strcmp(ir->name, "gl_FragColor") == 0) {
> + for (int i = 0; i< c->key.nr_color_regions; i++) {
> + this->outputs[i] = *reg;
> + }
> + } else if (strcmp(ir->name, "gl_FragData") == 0) {
> + for (int i = 0; i< c->key.nr_color_regions; i++) {
> + this->outputs[i] = *reg;
> + this->outputs[i].reg_offset += 4 * i;
> + }
> + } else if (strcmp(ir->name, "gl_FragDepth") == 0) {
> + this->frag_depth = ir;
> + } else {
> + assert(ir->location>= FRAG_RESULT_DATA0&&
> + ir->location< FRAG_RESULT_DATA0 + BRW_MAX_DRAW_BUFFERS);
>
> - if (ir->mode == ir_var_uniform) {
> + /* 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;
> + }
> + }
Instead of using strcmp, why not just use ir->location? This should
allow the gl_FragData and the general color output cases to be merged
(since they'll have the same locations).
> + } else if (ir->mode == ir_var_uniform) {
> int param_index = c->prog_data.nr_params;
>
> if (c->dispatch_width == 16) {
> @@ -1830,10 +1845,18 @@ fs_visitor::emit_interpolation_setup_gen6()
> }
>
> void
> -fs_visitor::emit_color_write(int index, int first_color_mrf, fs_reg color)
> +fs_visitor::emit_color_write(int target, int index, int first_color_mrf)
> {
> int reg_width = c->dispatch_width / 8;
> fs_inst *inst;
> + fs_reg color = outputs[target];
> + fs_reg mrf;
> +
> + /* If there's no color data to be written, skip it. */
> + if (color.file == BAD_FILE)
> + return;
> +
> + color.reg_offset += index;
>
> if (c->dispatch_width == 8 || intel->gen>= 6) {
> /* SIMD8 write looks like:
> @@ -1853,7 +1876,7 @@ fs_visitor::emit_color_write(int index, int first_color_mrf, fs_reg color)
> * m + 7: a1
> */
> inst = emit(BRW_OPCODE_MOV,
> - fs_reg(MRF, first_color_mrf + index * reg_width),
> + fs_reg(MRF, first_color_mrf + index * reg_width, color.type),
> color);
> inst->saturate = c->key.clamp_fragment_color;
> } else {
> @@ -1874,19 +1897,22 @@ fs_visitor::emit_color_write(int index, int first_color_mrf, fs_reg color)
> * destination + 4.
> */
> inst = emit(BRW_OPCODE_MOV,
> - fs_reg(MRF, BRW_MRF_COMPR4 + first_color_mrf + index),
> + fs_reg(MRF, BRW_MRF_COMPR4 + first_color_mrf + index,
> + color.type),
> color);
> inst->saturate = c->key.clamp_fragment_color;
> } else {
> push_force_uncompressed();
> - inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index),
> + inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index,
> + color.type),
> color);
> inst->saturate = c->key.clamp_fragment_color;
> pop_force_uncompressed();
>
> push_force_sechalf();
> color.sechalf = true;
> - inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index + 4),
> + inst = emit(BRW_OPCODE_MOV, fs_reg(MRF, first_color_mrf + index + 4,
> + color.type),
> color);
> inst->saturate = c->key.clamp_fragment_color;
> pop_force_sechalf();
> @@ -1956,27 +1982,12 @@ fs_visitor::emit_fb_writes()
> nr += reg_width;
> }
>
> - fs_reg color = reg_undef;
> - if (this->frag_color)
> - color = *(variable_storage(this->frag_color));
> - else if (this->frag_data) {
> - color = *(variable_storage(this->frag_data));
> - color.type = BRW_REGISTER_TYPE_F;
> - }
> -
> for (int target = 0; target< c->key.nr_color_regions; target++) {
> this->current_annotation = ralloc_asprintf(this->mem_ctx,
> "FB write target %d",
> target);
> - if (this->frag_color || this->frag_data) {
> - for (int i = 0; i< 4; i++) {
> - emit_color_write(i, color_mrf, color);
> - color.reg_offset++;
> - }
> - }
> -
> - if (this->frag_color)
> - color.reg_offset -= 4;
> + for (int i = 0; i< 4; i++)
> + emit_color_write(target, i, color_mrf);
>
> fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
> inst->target = target;
> @@ -1988,13 +1999,12 @@ fs_visitor::emit_fb_writes()
> }
>
> if (c->key.nr_color_regions == 0) {
> - if (c->key.alpha_test&& (this->frag_color || this->frag_data)) {
> + if (c->key.alpha_test) {
> /* If the alpha test is enabled but there's no color buffer,
> * we still need to send alpha out the pipeline to our null
> * renderbuffer.
> */
> - color.reg_offset += 3;
> - emit_color_write(3, color_mrf, color);
> + emit_color_write(0, 3, color_mrf);
> }
>
> fs_inst *inst = emit(FS_OPCODE_FB_WRITE);
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index d9d9414..f57b2e6 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -170,6 +170,7 @@ brw_type_for_base_type(const struct glsl_type *type)
> case GLSL_TYPE_UINT:
> return BRW_REGISTER_TYPE_UD;
> case GLSL_TYPE_ARRAY:
> + return brw_type_for_base_type(type->fields.array);
> case GLSL_TYPE_STRUCT:
> case GLSL_TYPE_SAMPLER:
> /* These should be overridden with the type of the member when
More information about the mesa-dev
mailing list