On Wed, Feb 7, 2018 at 12:24 PM, Francisco Jerez <curroje...@riseup.net> wrote:
> Anuj Phogat <anuj.pho...@gmail.com> writes: > > > On Wed, Feb 7, 2018 at 11:55 AM, Francisco Jerez <curroje...@riseup.net> > > wrote: > > > >> Anuj Phogat <anuj.pho...@gmail.com> writes: > >> > >> > On Wed, Feb 7, 2018 at 11:14 AM, Kenneth Graunke < > kenn...@whitecape.org> > >> > wrote: > >> > > >> >> On Tuesday, February 6, 2018 5:09:10 PM PST Anuj Phogat wrote: > >> >> > From Message Descriptor section in gfxspecs: > >> >> > "Memory fence messages without Commit Enable set do not return > >> >> > anything to the thread (response length is 0 and destination > >> >> > register is null)." > >> >> > > >> >> > It fixes a piglit GPU hang in simulation environment. > >> >> > Piglit test: arb_shader_image_load_store-shader-mem-barrier > >> >> > > >> >> > Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com> > >> >> > Cc: mesa-sta...@lists.freedesktop.org > >> >> > --- > >> >> > src/intel/compiler/brw_eu_emit.c | 8 +++++++- > >> >> > 1 file changed, 7 insertions(+), 1 deletion(-) > >> >> > > >> >> > diff --git a/src/intel/compiler/brw_eu_emit.c > >> >> b/src/intel/compiler/brw_eu_emit.c > >> >> > index 1fb9aab51c..c66b813af8 100644 > >> >> > --- a/src/intel/compiler/brw_eu_emit.c > >> >> > +++ b/src/intel/compiler/brw_eu_emit.c > >> >> > @@ -3290,7 +3290,13 @@ brw_memory_fence(struct brw_codegen *p, > >> >> > */ > >> >> > insn = next_insn(p, BRW_OPCODE_SEND); > >> >> > dst = retype(dst, BRW_REGISTER_TYPE_UW); > >> >> > - brw_set_dest(p, insn, dst); > >> >> > + > >> >> > + /* From Message Descriptor section in gfxspecs: > >> >> > + "Memory fence messages without Commit Enable set do not > >> return > >> >> > + anything to the thread (response length is 0 and > destination > >> >> > + register is null). > >> >> > + */ > >> >> > + brw_set_dest(p, insn, retype(vec1(brw_null_reg()), > >> >> BRW_REGISTER_TYPE_UW)); > >> >> > brw_set_src0(p, insn, dst); > >> >> > brw_set_memory_fence_message(p, insn, > >> GEN7_SFID_DATAPORT_DATA_CACHE, > >> >> > commit_enable); > >> >> > > >> >> > >> >> This seems wrong - it looks like you're doing this for messages with > >> >> commit enable set, which do return things... > >> >> > >> >> Right. I've to check for commit_enable to use null or non-null > >> destination > >> > here. I'll send out a v2. > >> > > >> > > >> > >> You can also just drop the patch, unless you want to change the > >> front-end in addition to stop allocating a destination for memory fences > >> on HSW-SKL in order to save a small amount of register pressure? > >> > > It also fixes a fulsim error other than reducing register pressure. > > On what platform? > I noticed this on Icelake. But, I'm sure it exists on CNL as well. > > > I'll look in to how to stop allocating dst register for memory fences > > and send out a v2. Thanks for the suggestion. > > > > In that case I think it would be nice to avoid duplicating the logic > used to decide whether to enable the commit bit or not. You can > allocate the register conditionally in the visitor (providing a null > register on HSW-SKL), and have brw_memory_fence() enable the commit bit > if and only if dst is not null, that way we have the guarantee that the > back-end and front-end are kept in sync. > sounds good to me. > > > > > > > >> > >> > > >> >> Cc'ing Curro. > >> >> > >> >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
