[Mesa-dev] [PATCH 05/13] i965: Fix the untyped surface opcodes to deal with indirect surface access.
Pohjolainen, Topi
topi.pohjolainen at intel.com
Thu Mar 12 10:39:50 PDT 2015
On Thu, Mar 12, 2015 at 07:25:34PM +0200, Francisco Jerez wrote:
> "Pohjolainen, Topi" <topi.pohjolainen at intel.com> writes:
>
> > On Fri, Feb 27, 2015 at 05:34:48PM +0200, Francisco Jerez wrote:
> >> Change brw_untyped_atomic() and brw_untyped_surface_read() to take the
> >> surface index as a register instead of a constant and to use
> >> brw_send_indirect_message() to emit the indirect variant of send with
> >> a dynamically calculated message descriptor. This will be required to
> >> support variable indexing of image arrays for
> >> ARB_shader_image_load_store.
> >> ---
> >> src/mesa/drivers/dri/i965/brw_eu.h | 10 +-
> >> src/mesa/drivers/dri/i965/brw_eu_emit.c | 158 +++++++++++++----------
> >> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 4 +-
> >> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 4 +-
> >> 4 files changed, 96 insertions(+), 80 deletions(-)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h b/src/mesa/drivers/dri/i965/brw_eu.h
> >> index 87a9f3f..9cc9123 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> >> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> >> @@ -398,18 +398,18 @@ void brw_CMP(struct brw_compile *p,
> >>
> >> void
> >> brw_untyped_atomic(struct brw_compile *p,
> >> - struct brw_reg dest,
> >> + struct brw_reg dst,
> >> struct brw_reg payload,
> >> + struct brw_reg surface,
> >> unsigned atomic_op,
> >> - unsigned bind_table_index,
> >> unsigned msg_length,
> >> bool response_expected);
> >>
> >> void
> >> brw_untyped_surface_read(struct brw_compile *p,
> >> - struct brw_reg dest,
> >> - struct brw_reg mrf,
> >> - unsigned bind_table_index,
> >> + struct brw_reg dst,
> >> + struct brw_reg payload,
> >> + struct brw_reg surface,
> >> unsigned msg_length,
> >> unsigned num_channels);
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> index 0b655d4..34695bf 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> >> @@ -2518,6 +2518,48 @@ brw_send_indirect_message(struct brw_compile *p,
> >> return setup;
> >> }
> >>
> >> +static struct brw_inst *
> >> +brw_send_indirect_surface_message(struct brw_compile *p,
> >> + unsigned sfid,
> >> + struct brw_reg dst,
> >> + struct brw_reg payload,
> >> + struct brw_reg surface,
> >> + unsigned message_len,
> >> + unsigned response_len,
> >> + bool header_present)
> >> +{
> >> + const struct brw_context *brw = p->brw;
> >> + struct brw_inst *insn;
> >> +
> >> + if (surface.file != BRW_IMMEDIATE_VALUE) {
> >> + struct brw_reg addr = retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD);
> >> +
> >> + brw_push_insn_state(p);
> >> + brw_set_default_access_mode(p, BRW_ALIGN_1);
> >> + brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> >> + brw_set_default_predicate_control(p, BRW_PREDICATE_NONE);
> >> +
> >> + /* Mask out invalid bits from the surface index to avoid hangs e.g. when
> >> + * some surface array is accessed out of bounds.
> >> + */
> >> + insn = brw_AND(p, addr,
> >> + suboffset(vec1(retype(surface, BRW_REGISTER_TYPE_UD)),
> >> + BRW_GET_SWZ(surface.dw1.bits.swizzle, 0)),
> >> + brw_imm_ud(0xff));
> >> +
> >> + brw_pop_insn_state(p);
> >> +
> >> + surface = addr;
> >
> > Also this patch didn't really need this branch at all. Instead it could
> > have been its own patch and here just:
> >
> > assert(surface.file == BRW_IMMEDIATE_VALUE);
> >
> The whole point of this patch is to fix untyped surface message sends to
> deal with non-immediate "surface" arguments. How could it not be
> needed?
I mean that the patch could be split in two parts: one doing the
re-organization and another adding the non-immediate branch to
brw_send_indirect_surface_message().
If I'm reading the patch right, brw_untyped_surface_read() and
brw_untyped_atomic() are only called with surface indices of immediate type.
Hence the added branch is not yet hit in runtime at this point.
More information about the mesa-dev
mailing list