[Mesa-stable] [Mesa-dev] [PATCH 2/2] intel/compiler: Use null destination register for memory fence messages

[Anuj Phogat]

On Wed, Feb 7, 2018 at 11:55 AM, Francisco Jerez <currojerez at riseup.net>
wrote:

> Anuj Phogat <anuj.phogat at gmail.com> writes:
>
> > On Wed, Feb 7, 2018 at 11:14 AM, Kenneth Graunke <kenneth at 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.phogat at gmail.com>
> >> > Cc: mesa-stable at 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. I'll
look in to how to stop allocating dst register for memory fences and
send out a v2. Thanks for the suggestion.  ​



>
> >
> >> Cc'ing Curro.
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-stable/attachments/20180207/b08918c8/attachment.html>