Iago Toral <ito...@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;
>>
>
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
