[Mesa-dev] [PATCH] i965/fs: Replace subreg_offset with brw_reg's subnr.
Francisco Jerez
currojerez at riseup.net
Thu Feb 11 23:33:29 UTC 2016
Kenneth Graunke <kenneth at whitecape.org> 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 <kenneth at whitecape.org>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160211/7c396779/attachment.sig>
More information about the mesa-dev
mailing list