[Mesa-dev] [PATCH 02/57] i965/vec4: Replace dst/src_reg::reg_offset with dst/src_reg::offset expressed in bytes.
Francisco Jerez
currojerez at riseup.net
Fri Sep 9 00:56:13 UTC 2016
Iago Toral <itoral at igalia.com> writes:
> On Wed, 2016-09-07 at 18:48 -0700, Francisco Jerez wrote:
> (...)
>> src/mesa/drivers/dri/i965/brw_ir_vec4.h | 4 +-
>> src/mesa/drivers/dri/i965/brw_vec4.cpp | 61
>> ++++++++++++----------
>> .../drivers/dri/i965/brw_vec4_cmod_propagation.cpp | 2 +-
>> .../drivers/dri/i965/brw_vec4_copy_propagation.cpp | 4 +-
>> .../drivers/dri/i965/brw_vec4_live_variables.h | 8 +--
>> src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 2 +-
>> .../drivers/dri/i965/brw_vec4_reg_allocate.cpp | 4 +-
>> src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 14 ++---
>> 8 files changed, 51 insertions(+), 48 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
>> b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
>> index 3813bb8..4f49428 100644
>> --- a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
>> +++ b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
>> @@ -65,7 +65,7 @@ offset(src_reg reg, unsigned delta)
>> {
>> assert(delta == 0 ||
>> (reg.file != ARF && reg.file != FIXED_GRF && reg.file !=
>> IMM));
>> - reg.reg_offset += delta;
>> + reg.offset += delta * (reg.file == UNIFORM ? 16 : REG_SIZE);
>> return reg;
>> }
>>
>> @@ -134,7 +134,7 @@ offset(dst_reg reg, unsigned delta)
>> {
>> assert(delta == 0 ||
>> (reg.file != ARF && reg.file != FIXED_GRF && reg.file !=
>> IMM));
>> - reg.reg_offset += delta;
>> + reg.offset += delta * (reg.file == UNIFORM ? 16 : REG_SIZE);
>> return reg;
>> }
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> index d52fdc0..dd058db 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> @@ -68,7 +68,7 @@ src_reg::src_reg()
>> src_reg::src_reg(struct ::brw_reg reg) :
>> backend_reg(reg)
>> {
>> - this->reg_offset = 0;
>> + this->offset = 0;
>> this->reladdr = NULL;
>> }
>>
>> @@ -125,7 +125,7 @@ dst_reg::dst_reg(enum brw_reg_file file, int nr,
>> brw_reg_type type,
>> dst_reg::dst_reg(struct ::brw_reg reg) :
>> backend_reg(reg)
>> {
>> - this->reg_offset = 0;
>> + this->offset = 0;
>> this->reladdr = NULL;
>> }
>>
>> @@ -395,7 +395,7 @@ vec4_visitor::opt_vector_float()
>> * sequence. Combine anything we've accumulated so far.
>> */
>> if (last_reg != inst->dst.nr ||
>> - last_reg_offset != inst->dst.reg_offset ||
>> + last_reg_offset != inst->dst.offset / REG_SIZE ||
>> last_reg_file != inst->dst.file ||
>> (vf > 0 && dest_type != need_type)) {
>>
>> @@ -439,7 +439,7 @@ vec4_visitor::opt_vector_float()
>> imm_inst[inst_count++] = inst;
>>
>> last_reg = inst->dst.nr;
>> - last_reg_offset = inst->dst.reg_offset;
>> + last_reg_offset = inst->dst.offset / REG_SIZE;
>> last_reg_file = inst->dst.file;
>> if (vf > 0)
>> dest_type = need_type;
>> @@ -539,8 +539,8 @@ vec4_visitor::split_uniform_registers()
>>
>> assert(!inst->src[i].reladdr);
>>
>> - inst->src[i].nr += inst->src[i].reg_offset;
>> - inst->src[i].reg_offset = 0;
>> + inst->src[i].nr += inst->src[i].offset / 16;
>> + inst->src[i].offset %= 16;
>> }
>> }
>> }
>> @@ -857,7 +857,7 @@
>> vec4_visitor::move_push_constants_to_pull_constants()
>>
>> inst->src[i].file = temp.file;
>> inst->src[i].nr = temp.nr;
>> - inst->src[i].reg_offset = temp.reg_offset;
>> + inst->src[i].offset %= 16;
>
> So it seems that temp.offset is going to be 0 here and that's why
> you're making this change. Looks good to me, just making sure that this
> is not something unintended since it is not quite following the pattern
> of directly translating the original code.
>
Yeah, that's right, I should probably have split this simplification
into a separate patch since it's apparently not fully obvious.
>> inst->src[i].reladdr = NULL;
>> }
>> }
>
> (...)
>
>> @@ -1831,7 +1834,7 @@ vec4_visitor::convert_to_hw_regs()
>> struct brw_reg reg;
>> switch (src.file) {
>> case VGRF:
>> - reg = brw_vec8_grf(src.nr + src.reg_offset, 0);
>> + reg = brw_vec8_grf(src.nr + src.offset / REG_SIZE, 0);
>> reg.type = src.type;
>> reg.swizzle = src.swizzle;
>> reg.abs = src.abs;
>> @@ -1840,8 +1843,8 @@ vec4_visitor::convert_to_hw_regs()
>>
>> case UNIFORM:
>> reg = stride(brw_vec4_grf(prog_data-
>> >base.dispatch_grf_start_reg +
>> - (src.nr + src.reg_offset) / 2,
>> - ((src.nr + src.reg_offset) %
>> 2) * 4),
>> + (src.nr + src.offset / 4) / 2,
>> + ((src.nr + src.offset / 4) %
>
> Shouldn't we divide by 16 instead of 4 here?
>
Yikes, looks like the reg_offset unit inconsistency striking back at me!
Luckily this code is removed later on in PATCH 47 which is why this
didn't show up during testing, but I've fixed up the register unit
locally in order to avoid a temporary regression. Good catch!
>> 2) * 4),
>> 0, 4, 1);
>> reg.type = src.type;
>> reg.swizzle = src.swizzle;
>> @@ -1887,14 +1890,14 @@ vec4_visitor::convert_to_hw_regs()
>>
>> switch (inst->dst.file) {
>> case VGRF:
>> - reg = brw_vec8_grf(dst.nr + dst.reg_offset, 0);
>> + reg = brw_vec8_grf(dst.nr + dst.offset / REG_SIZE, 0);
>> reg.type = dst.type;
>> reg.writemask = dst.writemask;
>> break;
>>
>
-------------- 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/20160908/f7755a39/attachment.sig>
More information about the mesa-dev
mailing list