Kenneth Graunke <[email protected]> writes: > On Thursday, February 11, 2016 1:49:21 PM PST Matt Turner wrote: >> --- >> src/mesa/drivers/dri/i965/brw_fs.cpp | 31 ++++++++++ > +----------- >> .../drivers/dri/i965/brw_fs_combine_constants.cpp | 13 +++++---- >> .../drivers/dri/i965/brw_fs_copy_propagation.cpp | 14 +++++----- >> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 2 +- >> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 9 ++----- >> src/mesa/drivers/dri/i965/brw_ir_fs.h | 13 +++------ >> 6 files changed, 35 insertions(+), 47 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/ > i965/brw_fs.cpp >> index 41a3f81..6ee590e 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp >> @@ -432,7 +432,6 @@ fs_reg::fs_reg(struct ::brw_reg reg) : >> backend_reg(reg) >> { >> this->reg_offset = 0; >> - this->subreg_offset = 0; >> this->reladdr = NULL; >> this->stride = 1; >> if (this->file == IMM && >> @@ -447,7 +446,6 @@ bool >> fs_reg::equals(const fs_reg &r) const >> { >> return (this->backend_reg::equals(r) && >> - subreg_offset == r.subreg_offset && >> !reladdr && !r.reladdr && >> stride == r.stride); >> } >> @@ -456,7 +454,8 @@ fs_reg & >> fs_reg::set_smear(unsigned subreg) >> { >> assert(file != ARF && file != FIXED_GRF && file != IMM); >> - subreg_offset = subreg * type_sz(type); >> + assert(subreg * type_sz(type) < (1 << 5)); /* subnr is 5 bits */ >> + subnr = subreg * type_sz(type); >> stride = 0; >> return *this; >> } >> @@ -1513,7 +1512,7 @@ fs_visitor::assign_curb_setup() >> assert(inst->src[i].stride == 0); >> inst->src[i] = byte_offset( >> retype(brw_reg, inst->src[i].type), >> - inst->src[i].subreg_offset); >> + inst->src[i].subnr); >> } >> } >> } >> @@ -1653,7 +1652,7 @@ fs_visitor::convert_attr_sources_to_hw_regs(fs_inst > *inst) >> unsigned width = inst->src[i].stride == 0 ? 1 : inst->exec_size; >> struct brw_reg reg = >> stride(byte_offset(retype(brw_vec8_grf(grf, 0), inst- >>src[i].type), >> - inst->src[i].subreg_offset), >> + inst->src[i].subnr), >> inst->exec_size * inst->src[i].stride, >> width, inst->src[i].stride); >> reg.abs = inst->src[i].abs; >> @@ -2597,7 +2596,7 @@ fs_visitor::compute_to_mrf() >> inst->dst.type != inst->src[0].type || >> inst->src[0].abs || inst->src[0].negate || >> !inst->src[0].is_contiguous() || >> - inst->src[0].subreg_offset) >> + inst->src[0].subnr) >> continue; >> >> /* Work out which hardware MRF registers are written by this >> @@ -3367,7 +3366,7 @@ fs_visitor::lower_integer_multiplication() >> assert(src1_1_w.stride == 1); >> src1_1_w.stride = 2; >> } >> - src1_1_w.subreg_offset += type_sz(BRW_REGISTER_TYPE_UW); >> + src1_1_w.subnr += type_sz(BRW_REGISTER_TYPE_UW); >> } >> ibld.MUL(low, inst->src[0], src1_0_w); >> ibld.MUL(high, inst->src[0], src1_1_w); >> @@ -3386,7 +3385,7 @@ fs_visitor::lower_integer_multiplication() >> assert(src0_1_w.stride == 1); >> src0_1_w.stride = 2; >> } >> - src0_1_w.subreg_offset += type_sz(BRW_REGISTER_TYPE_UW); >> + src0_1_w.subnr += type_sz(BRW_REGISTER_TYPE_UW); >> >> ibld.MUL(low, src0_0_w, inst->src[1]); >> ibld.MUL(high, src0_1_w, inst->src[1]); >> @@ -3394,14 +3393,14 @@ fs_visitor::lower_integer_multiplication() >> >> fs_reg dst = inst->dst; >> dst.type = BRW_REGISTER_TYPE_UW; >> - dst.subreg_offset = 2; >> + dst.subnr = 2; >> dst.stride = 2; >> >> high.type = BRW_REGISTER_TYPE_UW; >> high.stride = 2; >> >> low.type = BRW_REGISTER_TYPE_UW; >> - low.subreg_offset = 2; >> + low.subnr = 2; >> low.stride = 2; >> >> ibld.ADD(dst, low, high); >> @@ -4642,9 +4641,9 @@ fs_visitor::dump_instruction(backend_instruction > *be_inst, FILE *file) >> case VGRF: >> fprintf(file, "vgrf%d", inst->dst.nr); >> if (alloc.sizes[inst->dst.nr] != inst->regs_written || >> - inst->dst.subreg_offset) >> + inst->dst.subnr) >> fprintf(file, "+%d.%d", >> - inst->dst.reg_offset, inst->dst.subreg_offset); >> + inst->dst.reg_offset, inst->dst.subnr); >> break; >> case FIXED_GRF: >> fprintf(file, "g%d", inst->dst.nr); >> @@ -4698,9 +4697,9 @@ fs_visitor::dump_instruction(backend_instruction > *be_inst, FILE *file) >> case VGRF: >> fprintf(file, "vgrf%d", inst->src[i].nr); >> if (alloc.sizes[inst->src[i].nr] != (unsigned)inst->regs_read(i) > || >> - inst->src[i].subreg_offset) >> + inst->src[i].subnr) >> fprintf(file, "+%d.%d", inst->src[i].reg_offset, >> - inst->src[i].subreg_offset); >> + inst->src[i].subnr); >> break; >> case FIXED_GRF: >> fprintf(file, "g%d", inst->src[i].nr); >> @@ -4715,9 +4714,9 @@ fs_visitor::dump_instruction(backend_instruction > *be_inst, FILE *file) >> fprintf(file, "u%d", inst->src[i].nr + inst->src[i].reg_offset); >> if (inst->src[i].reladdr) { >> fprintf(file, "+reladdr"); >> - } else if (inst->src[i].subreg_offset) { >> + } else if (inst->src[i].subnr) { >> fprintf(file, "+%d.%d", inst->src[i].reg_offset, >> - inst->src[i].subreg_offset); >> + inst->src[i].subnr); >> } >> break; >> case BAD_FILE: >> diff --git a/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp b/src/ > mesa/drivers/dri/i965/brw_fs_combine_constants.cpp >> index d7a1456..f7c9786 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_fs_combine_constants.cpp >> @@ -121,7 +121,7 @@ struct imm { >> * The GRF register and subregister number where we've decided to store > the >> * constant value. >> */ >> - uint8_t subreg_offset; >> + uint8_t subnr; >> uint16_t nr; >> >> /** The number of coissuable instructions using this immediate. */ >> @@ -281,12 +281,11 @@ fs_visitor::opt_combine_constants() >> >> ibld.MOV(reg, brw_imm_f(imm->val)); >> imm->nr = reg.nr; >> - imm->subreg_offset = reg.subreg_offset; >> + imm->subnr = reg.subnr; >> >> - reg.subreg_offset += sizeof(float); >> - if ((unsigned)reg.subreg_offset == 8 * sizeof(float)) { >> + reg.subnr += sizeof(float); >> + if (reg.subnr == 0) { > > This was a bit tricky...8 * sizeof(float) == 32 == 0 because of the > 5-bit subnr field. But it looks correct. > > Thanks for getting rid of subreg_offset. > Would be really nice if we could also get rid of reg_offset as we're at it. reg and subreg_offset basically represent the same thing but with different units, couldn't we just have a single offset field in bytes? Should it be part of brw_reg or backend_reg? I think I would lean towards backend_reg. In that case does it make sense to move this into brw_reg now only to move it back to backend_reg later on?
> Reviewed-by: Kenneth Graunke <[email protected]> > _______________________________________________ > mesa-dev mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
