[Mesa-dev] [PATCH 02/13] i965: Change header_present to header_size in backend_instruction
Pohjolainen, Topi
topi.pohjolainen at intel.com
Fri Apr 3 00:58:35 PDT 2015
On Wed, Apr 01, 2015 at 06:19:13PM -0700, Jason Ekstrand wrote:
> ---
> src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp | 4 ++--
> src/mesa/drivers/dri/i965/brw_fs.cpp | 8 +++----
> src/mesa/drivers/dri/i965/brw_fs_cse.cpp | 2 +-
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 20 ++++++++--------
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 30 +++++++++++++-----------
> src/mesa/drivers/dri/i965/brw_shader.h | 3 ++-
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 2 +-
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 6 ++---
> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 14 +++++------
> 9 files changed, 46 insertions(+), 43 deletions(-)
>
I can't think the logic through all the way and therefore I need to ask.
I noticed that there is now a comparison checking for exact match of header
size. Before such would have passed as values two and one were both presented
with boolean true. Also message length changes at least in one occurrence as
well as parameter base offset (header size one vs. two).
So the question is that is this patch expected to change the runtime
behaviour, and if not, what guarantees that? In fact, were we doing something
wrong in the past because of this.
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> index 32919b1..c1b7609 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> @@ -88,7 +88,7 @@ brw_blorp_eu_emitter::emit_texture_lookup(const struct brw_reg &dst,
>
> inst->base_mrf = base_mrf;
> inst->mlen = msg_length;
> - inst->header_present = false;
> + inst->header_size = 0;
>
> insts.push_tail(inst);
> }
> @@ -104,7 +104,7 @@ brw_blorp_eu_emitter::emit_render_target_write(const struct brw_reg &src0,
> inst->src[0] = src0;
> inst->base_mrf = msg_reg_nr;
> inst->mlen = msg_length;
> - inst->header_present = use_header;
> + inst->header_size = use_header ? 2 : 0;
> inst->target = BRW_BLORP_RENDERBUFFER_BINDING_TABLE_INDEX;
>
> insts.push_tail(inst);
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 9c2ccce..852abbe 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -430,7 +430,7 @@ fs_visitor::VARYING_PULL_CONSTANT_LOAD(const fs_reg &dst,
>
> if (brw->gen < 7) {
> inst->base_mrf = 13;
> - inst->header_present = true;
> + inst->header_size = 1;
> if (brw->gen == 4)
> inst->mlen = 3;
> else
> @@ -478,7 +478,7 @@ fs_inst::equals(fs_inst *inst) const
> base_mrf == inst->base_mrf &&
> target == inst->target &&
> eot == inst->eot &&
> - header_present == inst->header_present &&
> + header_size == inst->header_size &&
> shadow_compare == inst->shadow_compare &&
> exec_size == inst->exec_size &&
> offset == inst->offset);
> @@ -2835,7 +2835,7 @@ fs_visitor::emit_repclear_shader()
> write->saturate = key->clamp_fragment_color;
> write->base_mrf = color_mrf;
> write->target = 0;
> - write->header_present = false;
> + write->header_size = 0;
> write->mlen = 1;
> } else {
> assume(key->nr_color_regions > 0);
> @@ -2844,7 +2844,7 @@ fs_visitor::emit_repclear_shader()
> write->saturate = key->clamp_fragment_color;
> write->base_mrf = base_mrf;
> write->target = i;
> - write->header_present = true;
> + write->header_size = 2;
> write->mlen = 3;
> }
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> index dd199fa..61837d2 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_cse.cpp
> @@ -177,7 +177,7 @@ instructions_match(fs_inst *a, fs_inst *b, bool *negate)
> a->regs_written == b->regs_written &&
> a->base_mrf == b->base_mrf &&
> a->eot == b->eot &&
> - a->header_present == b->header_present &&
> + a->header_size == b->header_size &&
> a->shadow_compare == b->shadow_compare)
> : true) &&
> operands_match(a, b, negate);
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index bd12147..f88c041 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -243,7 +243,7 @@ fs_generator::fire_fb_write(fs_inst *inst,
> 0,
> inst->eot,
> last_render_target,
> - inst->header_present);
> + inst->header_size != 0);
>
> brw_mark_surface_used(&prog_data->base, surf_index);
> }
> @@ -265,7 +265,7 @@ fs_generator::generate_fb_write(fs_inst *inst, struct brw_reg payload)
> /* Header is 2 regs, g0 and g1 are the contents. g0 will be implied
> * move, here's g1.
> */
> - if (inst->header_present) {
> + if (inst->header_size != 0) {
> brw_push_insn_state(p);
> brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
> @@ -381,7 +381,7 @@ fs_generator::generate_blorp_fb_write(fs_inst *inst)
> 0,
> true,
> true,
> - inst->header_present);
> + inst->header_size != 0);
> }
>
> /* Computes the integer pixel x,y values from the origin.
> @@ -676,7 +676,7 @@ fs_generator::generate_tex(fs_inst *inst, struct brw_reg dst, struct brw_reg src
> dst = vec16(dst);
> }
>
> - assert(brw->gen < 7 || !inst->header_present ||
> + assert(brw->gen < 7 || inst->header_size == 0 ||
> src.file == BRW_GENERAL_REGISTER_FILE);
>
> assert(sampler_index.type == BRW_REGISTER_TYPE_UD);
> @@ -685,7 +685,7 @@ fs_generator::generate_tex(fs_inst *inst, struct brw_reg dst, struct brw_reg src
> * we need to set it up explicitly and load the offset bitfield.
> * Otherwise, we can use an implied move from g0 to the first message reg.
> */
> - if (inst->header_present) {
> + if (inst->header_size != 0) {
> if (brw->gen < 6 && !inst->offset) {
> /* Set up an implied move from g0 to the MRF. */
> src = retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UW);
> @@ -733,7 +733,7 @@ fs_generator::generate_tex(fs_inst *inst, struct brw_reg dst, struct brw_reg src
> msg_type,
> rlen,
> inst->mlen,
> - inst->header_present,
> + inst->header_size != 0,
> simd_mode,
> return_format);
>
> @@ -773,7 +773,7 @@ fs_generator::generate_tex(fs_inst *inst, struct brw_reg dst, struct brw_reg src
> msg_type,
> rlen,
> inst->mlen /* mlen */,
> - inst->header_present /* header */,
> + inst->header_size != 0 /* header */,
> simd_mode,
> return_format);
>
> @@ -1110,7 +1110,7 @@ fs_generator::generate_varying_pull_constant_load(fs_inst *inst,
> struct brw_reg offset)
> {
> assert(brw->gen < 7); /* Should use the gen7 variant. */
> - assert(inst->header_present);
> + assert(inst->header_size != 0);
> assert(inst->mlen);
>
> assert(index.file == BRW_IMMEDIATE_VALUE &&
> @@ -1163,7 +1163,7 @@ fs_generator::generate_varying_pull_constant_load(fs_inst *inst,
> msg_type,
> rlen,
> inst->mlen,
> - inst->header_present,
> + inst->header_size != 0,
> simd_mode,
> return_format);
>
> @@ -1180,7 +1180,7 @@ fs_generator::generate_varying_pull_constant_load_gen7(fs_inst *inst,
> /* Varying-offset pull constant loads are treated as a normal expression on
> * gen7, so the fact that it's a send message is hidden at the IR level.
> */
> - assert(!inst->header_present);
> + assert(!inst->header_size != 0);
> assert(!inst->mlen);
> assert(index.type == BRW_REGISTER_TYPE_UD);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index e6fb0cb..0624528 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -1572,7 +1572,7 @@ fs_visitor::emit_texture_gen4(ir_texture_opcode op, fs_reg dst,
> fs_inst *inst = emit(opcode, dst, reg_undef, fs_reg(sampler));
> inst->base_mrf = base_mrf;
> inst->mlen = mlen;
> - inst->header_present = true;
> + inst->header_size = 1;
> inst->regs_written = simd16 ? 8 : 4;
>
> if (simd16) {
> @@ -1603,7 +1603,7 @@ fs_visitor::emit_texture_gen5(ir_texture_opcode op, fs_reg dst,
> bool has_offset)
> {
> int reg_width = dispatch_width / 8;
> - bool header_present = false;
> + bool header_size = 0;
>
> fs_reg message(MRF, 2, BRW_REGISTER_TYPE_F, dispatch_width);
> fs_reg msg_coords = message;
> @@ -1612,7 +1612,7 @@ fs_visitor::emit_texture_gen5(ir_texture_opcode op, fs_reg dst,
> /* The offsets set up by the ir_texture visitor are in the
> * m1 header, so we can't go headerless.
> */
> - header_present = true;
> + header_size = 1;
> message.reg--;
> }
>
> @@ -1715,7 +1715,7 @@ fs_visitor::emit_texture_gen5(ir_texture_opcode op, fs_reg dst,
> fs_inst *inst = emit(opcode, dst, reg_undef, fs_reg(sampler));
> inst->base_mrf = message.reg;
> inst->mlen = msg_end.reg - message.reg;
> - inst->header_present = header_present;
> + inst->header_size = header_size;
> inst->regs_written = 4 * reg_width;
>
> if (inst->mlen > MAX_SAMPLER_MESSAGE_SIZE) {
> @@ -1744,7 +1744,7 @@ fs_visitor::emit_texture_gen7(ir_texture_opcode op, fs_reg dst,
> fs_reg offset_value)
> {
> int reg_width = dispatch_width / 8;
> - bool header_present = false;
> + bool header_size = 0;
>
> fs_reg *sources = ralloc_array(mem_ctx, fs_reg, MAX_SAMPLER_MESSAGE_SIZE);
> for (int i = 0; i < MAX_SAMPLER_MESSAGE_SIZE; i++) {
> @@ -1764,7 +1764,7 @@ fs_visitor::emit_texture_gen7(ir_texture_opcode op, fs_reg dst,
> * The sampler index is only 4-bits, so for larger sampler numbers we
> * need to offset the Sampler State Pointer in the header.
> */
> - header_present = true;
> + header_size = 1;
> sources[0] = fs_reg(GRF, alloc.allocate(1), BRW_REGISTER_TYPE_UD);
> length++;
> }
> @@ -1903,7 +1903,7 @@ fs_visitor::emit_texture_gen7(ir_texture_opcode op, fs_reg dst,
>
> int mlen;
> if (reg_width == 2)
> - mlen = length * reg_width - header_present;
> + mlen = length * reg_width - header_size;
> else
> mlen = length * reg_width;
>
> @@ -1935,7 +1935,7 @@ fs_visitor::emit_texture_gen7(ir_texture_opcode op, fs_reg dst,
> fs_inst *inst = emit(opcode, dst, src_payload, sampler);
> inst->base_mrf = -1;
> inst->mlen = mlen;
> - inst->header_present = header_present;
> + inst->header_size = header_size;
> inst->regs_written = 4 * reg_width;
>
> if (inst->mlen > MAX_SAMPLER_MESSAGE_SIZE) {
> @@ -2081,7 +2081,7 @@ fs_visitor::emit_mcs_fetch(fs_reg coordinate, int components, fs_reg sampler)
> fs_inst *inst = emit(SHADER_OPCODE_TXF_MCS, dest, payload, sampler);
> inst->base_mrf = -1;
> inst->mlen = components * reg_width;
> - inst->header_present = false;
> + inst->header_size = 0;
> inst->regs_written = 4 * reg_width; /* we only care about one reg of
> * response, but the sampler always
> * writes 4/8
> @@ -3299,7 +3299,7 @@ fs_visitor::emit_dummy_fs()
> write->base_mrf = 2;
> write->mlen = 4 * reg_width;
> } else {
> - write->header_present = true;
> + write->header_size = 2;
> write->base_mrf = 0;
> write->mlen = 2 + 4 * reg_width;
> }
> @@ -3596,7 +3596,7 @@ fs_visitor::emit_single_fb_write(fs_reg color0, fs_reg color1,
> brw_wm_prog_key *key = (brw_wm_prog_key*) this->key;
>
> this->current_annotation = "FB write header";
> - bool header_present = true;
> + int header_size = 2;
> int reg_size = dispatch_width / 8;
>
> /* We can potentially have a message length of up to 15, so we have to set
> @@ -3616,12 +3616,14 @@ fs_visitor::emit_single_fb_write(fs_reg color0, fs_reg color1,
> (brw->is_haswell || brw->gen >= 8 || !prog_data->uses_kill) &&
> color1.file == BAD_FILE &&
> key->nr_color_regions == 1) {
> - header_present = false;
> + header_size = 0;
> }
>
> - if (header_present)
> + if (header_size != 0) {
> + assert(header_size == 2);
> /* Allocate 2 registers for a header */
> length += 2;
> + }
>
> if (payload.aa_dest_stencil_reg) {
> sources[length] = fs_reg(GRF, alloc.allocate(1));
> @@ -3720,7 +3722,7 @@ fs_visitor::emit_single_fb_write(fs_reg color0, fs_reg color1,
> }
>
> write->mlen = load->regs_written;
> - write->header_present = header_present;
> + write->header_size = header_size;
> if (prog_data->uses_kill) {
> write->predicate = BRW_PREDICATE_NORMAL;
> write->flag_subreg = 1;
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.h b/src/mesa/drivers/dri/i965/brw_shader.h
> index 8a3263e..c3738f8 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.h
> +++ b/src/mesa/drivers/dri/i965/brw_shader.h
> @@ -140,12 +140,13 @@ struct backend_instruction {
> bool no_dd_check:1;
> bool saturate:1;
> bool shadow_compare:1;
> - bool header_present:1;
>
> /* Chooses which flag subregister (f0.0 or f0.1) is used for conditional
> * mod and predication.
> */
> unsigned flag_subreg:1;
> +
> + uint8_t header_size;
> };
>
> #ifdef __cplusplus
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 480e50c..4e294d1 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -303,7 +303,7 @@ vec4_visitor::implied_mrf_writes(vec4_instruction *inst)
> case SHADER_OPCODE_TXS:
> case SHADER_OPCODE_TG4:
> case SHADER_OPCODE_TG4_OFFSET:
> - return inst->header_present ? 1 : 0;
> + return inst->header_size;
> case SHADER_OPCODE_UNTYPED_ATOMIC:
> case SHADER_OPCODE_UNTYPED_SURFACE_READ:
> return 0;
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index 9714785..be87811 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -325,7 +325,7 @@ vec4_generator::generate_tex(vec4_instruction *inst,
> * to set it up explicitly and load the offset bitfield. Otherwise, we can
> * use an implied move from g0 to the first message register.
> */
> - if (inst->header_present) {
> + if (inst->header_size != 0) {
> if (brw->gen < 6 && !inst->offset) {
> /* Set up an implied move from g0 to the MRF. */
> src = brw_vec8_grf(0, 0);
> @@ -390,7 +390,7 @@ vec4_generator::generate_tex(vec4_instruction *inst,
> msg_type,
> 1, /* response length */
> inst->mlen,
> - inst->header_present,
> + inst->header_size != 0,
> BRW_SAMPLER_SIMD_MODE_SIMD4X2,
> return_format);
>
> @@ -430,7 +430,7 @@ vec4_generator::generate_tex(vec4_instruction *inst,
> msg_type,
> 1 /* rlen */,
> inst->mlen /* mlen */,
> - inst->header_present /* header */,
> + inst->header_size != 0 /* header */,
> BRW_SAMPLER_SIMD_MODE_SIMD4X2,
> return_format);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index ca1a995..ec7c1ff 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -50,7 +50,7 @@ vec4_instruction::vec4_instruction(enum opcode opcode, const dst_reg &dst,
> this->shadow_compare = false;
> this->ir = NULL;
> this->urb_write_flags = BRW_URB_WRITE_NO_FLAGS;
> - this->header_present = false;
> + this->header_size = 0;
> this->flag_subreg = 0;
> this->mlen = 0;
> this->base_mrf = 0;
> @@ -2596,19 +2596,19 @@ vec4_visitor::visit(ir_texture *ir)
> * - Gather channel selection
> * - Sampler indices too large to fit in a 4-bit value.
> */
> - inst->header_present =
> - brw->gen < 5 || brw->gen >= 9 ||
> - inst->offset != 0 || ir->op == ir_tg4 ||
> - is_high_sampler(brw, sampler_reg);
> + inst->header_size =
> + (brw->gen < 5 || brw->gen >= 9 ||
> + inst->offset != 0 || ir->op == ir_tg4 ||
> + is_high_sampler(brw, sampler_reg)) ? 1 : 0;
> inst->base_mrf = 2;
> - inst->mlen = inst->header_present + 1; /* always at least one */
> + inst->mlen = inst->header_size + 1; /* always at least one */
> inst->dst.writemask = WRITEMASK_XYZW;
> inst->shadow_compare = ir->shadow_comparitor != NULL;
>
> inst->src[1] = sampler_reg;
>
> /* MRF for the first parameter */
> - int param_base = inst->base_mrf + inst->header_present;
> + int param_base = inst->base_mrf + inst->header_size;
>
> if (ir->op == ir_txs || ir->op == ir_query_levels) {
> int writemask = brw->gen == 4 ? WRITEMASK_W : WRITEMASK_X;
> --
> 2.3.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list