On Thu, Mar 12, 2015 at 07:25:34PM +0200, Francisco Jerez wrote: > "Pohjolainen, Topi" <topi.pohjolai...@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. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev