[Mesa-dev] [Mesa-stable] [PATCH] ac/nir: Make the GFX9 buffer size fix apply to image loads/atomics too.

Juan A. Suarez Romero jasuarez at igalia.com
Wed Apr 18 07:53:26 UTC 2018


On Tue, 2018-04-17 at 05:56 +0200, Bas Nieuwenhuizen wrote:
> On Mon, Apr 16, 2018 at 1:17 PM, Juan A. Suarez Romero
> <jasuarez at igalia.com> wrote:
> > On Mon, 2018-04-16 at 00:09 +0200, Bas Nieuwenhuizen wrote:
> > > No clue how I missed those ...
> > > 
> > > Fixes: 4503ff760c "ac/nir: Add workaround for GFX9 buffer views."
> > > CC: <mesa-stable at lists.freedesktop.org>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105320
> > 
> > 
> > Hi, Bas!
> > 
> > This commit is a candidate for next 17.3 and 18.0 stable releases.
> > 
> > 
> > Regarding the next 17.3.9 release, which should be released today, as this is
> > the last one of 17.3, I understand you want this patch as part of the release,
> > right?
> 
> Its fine not to. The urgency isn't there compared to the original fix,
> and of course being the last release also carries the additional risk
> of not being able to fix it if the patch turns out to break something
> unexpected. I did not expect it to make it in.
> 

Thanks. I'll leave it out to avoid any additional risk.

> > 
> > If that's the case, I cherry-picked the commit and solved some trivial
> > conflicts. You can check to verify I correctly solved them at:
> > 
> > https://github.com/Igalia/release-mesa/commit/37ad9fc7c6cecdb8a99d071ca6fdc2d663
> > 7501a8
> > 
> > 
> > Related with that commit, maybe you want to ensure that this one also is
> > correct:
> > 
> > https://github.com/Igalia/release-mesa/commit/51b4bdc7761b30a56299ee80f51521151d
> > 4eec47
> 
> Looks correct to me, but I don't see any diffs with the backport I provided?

Yes, I just plainly applied the backport, which happened without any conflict. I
just pointed this commit because the backport was for 17.3, so just to double
check that it is valid also for 18.0.

> > 
> > 
> > 
> > Regarding the next 18.0.1, is it fine if we post-pone this patch for next
> > 18.0.2, or do you think this must be added in 18.0.1?
> 
> 18.0.2 is fine for the same reason as above.
> 

Nice.

Thank you.

	J.A.


> > 
> > 
> >         J.A.
> > 
> > 
> > 
> > > ---
> > >  src/amd/common/ac_nir_to_llvm.c | 39 +++++++++++++++++++--------------
> > >  1 file changed, 22 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
> > > index 3a3aa72988..45405d30fe 100644
> > > --- a/src/amd/common/ac_nir_to_llvm.c
> > > +++ b/src/amd/common/ac_nir_to_llvm.c
> > > @@ -2191,6 +2191,25 @@ static LLVMValueRef get_image_coords(struct ac_nir_context *ctx,
> > >       return res;
> > >  }
> > > 
> > > +static LLVMValueRef get_image_buffer_descriptor(struct ac_nir_context *ctx,
> > > +                                                const nir_intrinsic_instr *instr, bool write)
> > > +{
> > > +     LLVMValueRef rsrc = get_sampler_desc(ctx, instr->variables[0], AC_DESC_BUFFER, NULL, true, write);
> > > +     if (ctx->abi->gfx9_stride_size_workaround) {
> > > +             LLVMValueRef elem_count = LLVMBuildExtractElement(ctx->ac.builder, rsrc, LLVMConstInt(ctx->ac.i32, 2, 0), "");
> > > +             LLVMValueRef stride = LLVMBuildExtractElement(ctx->ac.builder, rsrc, LLVMConstInt(ctx->ac.i32, 1, 0), "");
> > > +             stride = LLVMBuildLShr(ctx->ac.builder, stride, LLVMConstInt(ctx->ac.i32, 16, 0), "");
> > > +
> > > +             LLVMValueRef new_elem_count = LLVMBuildSelect(ctx->ac.builder,
> > > +                                                           LLVMBuildICmp(ctx->ac.builder, LLVMIntUGT, elem_count, stride, ""),
> > > +                                                           elem_count, stride, "");
> > > +
> > > +             rsrc = LLVMBuildInsertElement(ctx->ac.builder, rsrc, new_elem_count,
> > > +                                           LLVMConstInt(ctx->ac.i32, 2, 0), "");
> > > +     }
> > > +     return rsrc;
> > > +}
> > > +
> > >  static LLVMValueRef visit_image_load(struct ac_nir_context *ctx,
> > >                                    const nir_intrinsic_instr *instr)
> > >  {
> > > @@ -2211,7 +2230,7 @@ static LLVMValueRef visit_image_load(struct ac_nir_context *ctx,
> > >               unsigned num_channels = util_last_bit(mask);
> > >               LLVMValueRef rsrc, vindex;
> > > 
> > > -             rsrc = get_sampler_desc(ctx, instr->variables[0], AC_DESC_BUFFER, NULL, true, false);
> > > +             rsrc = get_image_buffer_descriptor(ctx, instr, false);
> > >               vindex = LLVMBuildExtractElement(ctx->ac.builder, get_src(ctx, instr->src[0]),
> > >                                                ctx->ac.i32_0, "");
> > > 
> > > @@ -2262,20 +2281,7 @@ static void visit_image_store(struct ac_nir_context *ctx,
> > >               glc = ctx->ac.i1true;
> > > 
> > >       if (dim == GLSL_SAMPLER_DIM_BUF) {
> > > -             LLVMValueRef rsrc = get_sampler_desc(ctx, instr->variables[0], AC_DESC_BUFFER, NULL, true, true);
> > > -
> > > -             if (ctx->abi->gfx9_stride_size_workaround) {
> > > -                     LLVMValueRef elem_count = LLVMBuildExtractElement(ctx->ac.builder, rsrc, LLVMConstInt(ctx->ac.i32, 2, 0), "");
> > > -                     LLVMValueRef stride = LLVMBuildExtractElement(ctx->ac.builder, rsrc, LLVMConstInt(ctx->ac.i32, 1, 0), "");
> > > -                     stride = LLVMBuildLShr(ctx->ac.builder, stride, LLVMConstInt(ctx->ac.i32, 16, 0), "");
> > > -
> > > -                     LLVMValueRef new_elem_count = LLVMBuildSelect(ctx->ac.builder,
> > > -                                                                   LLVMBuildICmp(ctx->ac.builder, LLVMIntUGT, elem_count, stride, ""),
> > > -                                                                   elem_count, stride, "");
> > > -
> > > -                     rsrc = LLVMBuildInsertElement(ctx->ac.builder, rsrc, new_elem_count,
> > > -                                                   LLVMConstInt(ctx->ac.i32, 2, 0), "");
> > > -             }
> > > +             LLVMValueRef rsrc = get_image_buffer_descriptor(ctx, instr, true);
> > > 
> > >               params[0] = ac_to_float(&ctx->ac, get_src(ctx, instr->src[2])); /* data */
> > >               params[1] = rsrc;
> > > @@ -2360,8 +2366,7 @@ static LLVMValueRef visit_image_atomic(struct ac_nir_context *ctx,
> > >       params[param_count++] = get_src(ctx, instr->src[2]);
> > > 
> > >       if (glsl_get_sampler_dim(type) == GLSL_SAMPLER_DIM_BUF) {
> > > -             params[param_count++] = get_sampler_desc(ctx, instr->variables[0], AC_DESC_BUFFER,
> > > -                                                      NULL, true, true);
> > > +             params[param_count++] = get_image_buffer_descriptor(ctx, instr, true);
> > >               params[param_count++] = LLVMBuildExtractElement(ctx->ac.builder, get_src(ctx, instr->src[0]),
> > >                                                               ctx->ac.i32_0, ""); /* vindex */
> > >               params[param_count++] = ctx->ac.i32_0; /* voffset */
> 
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable


More information about the mesa-dev mailing list