On Wed, 2016-10-26 at 13:13 -0700, Francisco Jerez wrote: > Iago Toral Quiroga <[email protected]> writes: > > > > > In a later patch we want to change the semantics of offset() to be > > in terms > > of SIMD width and scalar channels so it is consistent with the > > definition > > of the same helper in the scalar backend. However, some uses of > > offset() > > in the vec4 backend do not operate naturally in terms of these > > semantics. In these cases it is more natural to use the > > byte_offset() helper > > instead. > > --- > > src/mesa/drivers/dri/i965/brw_vec4_cse.cpp | 10 > > ++++++---- > > .../drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp | 16 > > +++++++++++----- > > src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp | 13 > > +++++++++---- > > src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp | 3 ++- > > src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 2 +- > > 5 files changed, 29 insertions(+), 15 deletions(-) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp > > b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp > > index 1b91db9..bf7e4a4 100644 > > --- a/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_vec4_cse.cpp > > @@ -182,8 +182,9 @@ vec4_visitor::opt_cse_local(bblock_t *block) > > NULL), inst->dst.type); > > > > for (unsigned i = 0; i < regs_written(entry- > > >generator); ++i) { > > - vec4_instruction *copy = MOV(offset(entry- > > >generator->dst, i), > > - offset(entry->tmp, > > i)); > > + vec4_instruction *copy = > > + MOV(byte_offset(entry->generator->dst, i * > > REG_SIZE), > > + byte_offset(entry->tmp, i * REG_SIZE)); > > copy->force_writemask_all = > > entry->generator->force_writemask_all; > > entry->generator->insert_after(block, copy); > > @@ -197,8 +198,9 @@ vec4_visitor::opt_cse_local(bblock_t *block) > > assert(inst->dst.type == entry->tmp.type); > > > > for (unsigned i = 0; i < regs_written(inst); ++i) { > > - vec4_instruction *copy = MOV(offset(inst->dst, > > i), > > - offset(entry->tmp, > > i)); > > + vec4_instruction *copy = > > + MOV(byte_offset(inst->dst, i * REG_SIZE), > > + byte_offset(entry->tmp, i * REG_SIZE)); > > copy->force_writemask_all = inst- > > >force_writemask_all; > > inst->insert_before(block, copy); > These last two hunks seem rather suspect... They're emitting a MOV > instruction for each register written even though you'd need one for > each SIMD-wide vector written. Seems like the loop upper bound will > need to be changed to 'inst->size_written / component_size' and the > MOV > arguments will need to be changed back to use offset() for DF > support.
Right, I noticed this too and changed it in the fp64 series, see: https://lists.freedesktop.org/archives/mesa-dev/2016-October/131379.htm l I still have to rebase that on top of this series though. > That said this keeps the current behavior unchanged so it looks okay > to > me for the moment: > > Reviewed-by: Francisco Jerez <[email protected]> > > > > > } > > diff --git > > a/src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp > > b/src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp > > index 50706a9..916a3fb 100644 > > --- a/src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_vec4_dead_code_eliminate.cpp > > @@ -61,8 +61,9 @@ vec4_visitor::dead_code_eliminate() > > if (inst->dst.file == VGRF) { > > for (unsigned i = 0; i < regs_written(inst); i++) { > > for (int c = 0; c < 4; c++) > > - result_live[c] |= BITSET_TEST( > > - live, var_from_reg(alloc, offset(inst- > > >dst, i), c)); > > + result_live[c] |= BITSET_TEST(live, > > + var_from_reg(alloc, > > + byte_offset(inst->dst, i * > > REG_SIZE), c)); > > } > > } else { > > for (unsigned c = 0; c < 4; c++) > > @@ -113,8 +114,11 @@ vec4_visitor::dead_code_eliminate() > > for (unsigned i = 0; i < regs_written(inst); i++) { > > for (int c = 0; c < 4; c++) { > > if (inst->dst.writemask & (1 << c)) { > > - BITSET_CLEAR(live, var_from_reg(alloc, > > - offset(inst- > > >dst, i), c)); > > + BITSET_CLEAR(live, > > + var_from_reg(alloc, > > + byte_offset(inst- > > >dst, > > + i * > > REG_SIZE), > > + c)); > > } > > } > > } > > @@ -135,7 +139,9 @@ vec4_visitor::dead_code_eliminate() > > for (unsigned j = 0; j < regs_read(inst, i); j++) { > > for (int c = 0; c < 4; c++) { > > BITSET_SET(live, var_from_reg(alloc, > > - offset(inst- > > >src[i], j), c)); > > + byte_offset(ins > > t->src[i], > > + j * > > REG_SIZE), > > + c)); > > } > > } > > } > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp > > b/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp > > index 20344ed..b70d6c2 100644 > > --- a/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_vec4_live_variables.cpp > > @@ -79,7 +79,9 @@ vec4_live_variables::setup_def_use() > > for (unsigned j = 0; j < regs_read(inst, i); j++) { > > for (int c = 0; c < 4; c++) { > > const unsigned v = > > - var_from_reg(alloc, offset(inst->src[i], > > j), c); > > + var_from_reg(alloc, > > + byte_offset(inst->src[i], j * > > REG_SIZE), > > + c); > > if (!BITSET_TEST(bd->def, v)) > > BITSET_SET(bd->use, v); > > } > > @@ -103,7 +105,8 @@ vec4_live_variables::setup_def_use() > > for (int c = 0; c < 4; c++) { > > if (inst->dst.writemask & (1 << c)) { > > const unsigned v = > > - var_from_reg(alloc, offset(inst->dst, i), > > c); > > + var_from_reg(alloc, > > + byte_offset(inst->dst, i * > > REG_SIZE), c); > > if (!BITSET_TEST(bd->use, v)) > > BITSET_SET(bd->def, v); > > } > > @@ -260,7 +263,8 @@ vec4_visitor::calculate_live_intervals() > > for (unsigned j = 0; j < regs_read(inst, i); j++) { > > for (int c = 0; c < 4; c++) { > > const unsigned v = > > - var_from_reg(alloc, offset(inst->src[i], j), > > c); > > + var_from_reg(alloc, > > + byte_offset(inst->src[i], j * > > REG_SIZE), c); > > start[v] = MIN2(start[v], ip); > > end[v] = ip; > > } > > @@ -273,7 +277,8 @@ vec4_visitor::calculate_live_intervals() > > for (int c = 0; c < 4; c++) { > > if (inst->dst.writemask & (1 << c)) { > > const unsigned v = > > - var_from_reg(alloc, offset(inst->dst, i), c); > > + var_from_reg(alloc, > > + byte_offset(inst->dst, i * > > REG_SIZE), c); > > start[v] = MIN2(start[v], ip); > > end[v] = ip; > > } > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp > > b/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp > > index 498fb7c..95f5c33 100644 > > --- a/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp > > @@ -240,7 +240,8 @@ vec4_tcs_visitor::emit_urb_write(const src_reg > > &value, > > inst = emit(TCS_OPCODE_SET_OUTPUT_URB_OFFSETS, > > dst_reg(message), > > brw_imm_ud(writemask), indirect_offset); > > inst->force_writemask_all = true; > > - inst = emit(MOV(offset(dst_reg(retype(message, value.type)), > > 1), value)); > > + inst = emit(MOV(byte_offset(dst_reg(retype(message, > > value.type)), REG_SIZE), > > + value)); > > inst->force_writemask_all = true; > > > > inst = emit(TCS_OPCODE_URB_WRITE, dst_null_f(), message); > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > > b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > > index 3e785bc..814d624 100644 > > --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > > +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > > @@ -781,7 +781,7 @@ > > vec4_visitor::emit_pull_constant_load_reg(dst_reg dst, > > else > > emit(pull); > > > > - dst_reg index_reg = retype(offset(dst_reg(header), 1), > > + dst_reg index_reg = retype(byte_offset(dst_reg(header), > > REG_SIZE), > > offset_reg.type); > > pull = MOV(writemask(index_reg, WRITEMASK_X), offset_reg); > > _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
