[Mesa-dev] [PATCH 2/3] i965/vec4: use byte_offset() instead of offset()
Iago Toral
itoral at igalia.com
Thu Oct 27 06:51:33 UTC 2016
On Wed, 2016-10-26 at 13:13 -0700, Francisco Jerez wrote:
> Iago Toral Quiroga <itoral at igalia.com> 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 <currojerez at riseup.net>
>
> >
> > }
> > 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);
> >
More information about the mesa-dev
mailing list