[Mesa-dev] [Mesa-stable] [PATCH 3/4] ac/nir: fix 64-bit shifts

[Andres Gomez]

I see.

I'll leave it out, then.

Thanks for the clarification! ☺


On Wed, 2017-08-16 at 20:58 -0400, Connor Abbott wrote:
> I didn't enable the Int64 capability for radv until the next patch in
> this series, so the problem this fixes was never exposed to users at
> all. I just ran into it when enabling VK_EXT_shader_ballot -- the
> extension doesn't require it, but the only way to generate SPIR-V
> using it with glslang is with the ARB_shader_ballot GLSL extension
> that does use 64-bit integers.
> 
> P.S. it would be nice to have some Int64 tests in the Vulkan CTS so we
> can catch stuff like this easier.
> 
> On Wed, Aug 16, 2017 at 8:49 PM, Andres Gomez <agomez at igalia.com> wrote:
> > Connor, it looks like we could want this patch in 17.1 (?)
> > 
> > On Fri, 2017-06-30 at 19:56 -0700, Connor Abbott wrote:
> > > From: Connor Abbott <cwabbott0 at gmail.com>
> > > 
> > > NIR always makes the shift amount 32 bits, but LLVM asserts if the two
> > > sources aren't the same type. Zero-extend the shift amount to make LLVM
> > > happy.
> > > 
> > > Signed-off-by: Connor Abbott <cwabbott0 at gmail.com>
> > > ---
> > >  src/amd/common/ac_nir_to_llvm.c | 15 ++++++++++++---
> > >  1 file changed, 12 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
> > > index 88f3f44..e72747a 100644
> > > --- a/src/amd/common/ac_nir_to_llvm.c
> > > +++ b/src/amd/common/ac_nir_to_llvm.c
> > > @@ -1621,13 +1621,22 @@ static void visit_alu(struct nir_to_llvm_context *ctx, const nir_alu_instr *inst
> > >               result = LLVMBuildXor(ctx->builder, src[0], src[1], "");
> > >               break;
> > >       case nir_op_ishl:
> > > -             result = LLVMBuildShl(ctx->builder, src[0], src[1], "");
> > > +             result = LLVMBuildShl(ctx->builder, src[0],
> > > +                                   LLVMBuildZExt(ctx->builder, src[1],
> > > +                                                 LLVMTypeOf(src[0]), ""),
> > > +                                   "");
> > >               break;
> > >       case nir_op_ishr:
> > > -             result = LLVMBuildAShr(ctx->builder, src[0], src[1], "");
> > > +             result = LLVMBuildAShr(ctx->builder, src[0],
> > > +                                    LLVMBuildZExt(ctx->builder, src[1],
> > > +                                                  LLVMTypeOf(src[0]), ""),
> > > +                                    "");
> > >               break;
> > >       case nir_op_ushr:
> > > -             result = LLVMBuildLShr(ctx->builder, src[0], src[1], "");
> > > +             result = LLVMBuildLShr(ctx->builder, src[0],
> > > +                                    LLVMBuildZExt(ctx->builder, src[1],
> > > +                                                  LLVMTypeOf(src[0]), ""),
> > > +                                    "");
> > >               break;
> > >       case nir_op_ilt:
> > >               result = emit_int_cmp(&ctx->ac, LLVMIntSLT, src[0], src[1]);
> > 
> > --
> > Br,
> > 
> > Andres
> 
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
-- 
Br,

Andres