[Mesa-dev] [PATCH] ac/nir: fix intrinsic names for atomic operations with LLVM 9+

[Bas Nieuwenhuizen]

r-b

On Mon, Apr 8, 2019 at 12:36 PM Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
>
>
> On 4/8/19 12:32 PM, Erik Faye-Lund wrote:
> > On Mon, 2019-04-08 at 11:39 +0200, Samuel Pitoiset wrote:
> >> This fixes the following LLVM error when using RADV_DEBUG=checkir:
> >> Intrinsic name not mangled correctly for type arguments! Should be:
> >> llvm.amdgcn.buffer.atomic.add.i32
> >> i32 (i32, <4 x i32>, i32, i32, i1)* @llvm.amdgcn.buffer.atomic.add
> >>
> >> The cmpswap operation still uses the old intrinsic.
> >>
> >> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> >> ---
> >>   src/amd/common/ac_nir_to_llvm.c | 32 +++++++++++++++++++++--------
> >> ---
> >>   1 file changed, 21 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/src/amd/common/ac_nir_to_llvm.c
> >> b/src/amd/common/ac_nir_to_llvm.c
> >> index 6739551ca26..cc819286c65 100644
> >> --- a/src/amd/common/ac_nir_to_llvm.c
> >> +++ b/src/amd/common/ac_nir_to_llvm.c
> >> @@ -1679,7 +1679,8 @@ static void visit_store_ssbo(struct
> >> ac_nir_context *ctx,
> >>   static LLVMValueRef visit_atomic_ssbo(struct ac_nir_context *ctx,
> >>                                         const nir_intrinsic_instr
> >> *instr)
> >>   {
> >> -    const char *name;
> >> +    const char *op;
> >> +    char name[64];
> >>      LLVMValueRef params[6];
> >>      int arg_count = 0;
> >>
> >> @@ -1696,39 +1697,48 @@ static LLVMValueRef visit_atomic_ssbo(struct
> >> ac_nir_context *ctx,
> >>
> >>      switch (instr->intrinsic) {
> >>      case nir_intrinsic_ssbo_atomic_add:
> >> -            name = "llvm.amdgcn.buffer.atomic.add";
> >> +            op = "add";
> >>              break;
> >>      case nir_intrinsic_ssbo_atomic_imin:
> >> -            name = "llvm.amdgcn.buffer.atomic.smin";
> >> +            op = "smin";
> >>              break;
> >>      case nir_intrinsic_ssbo_atomic_umin:
> >> -            name = "llvm.amdgcn.buffer.atomic.umin";
> >> +            op = "umin";
> >>              break;
> >>      case nir_intrinsic_ssbo_atomic_imax:
> >> -            name = "llvm.amdgcn.buffer.atomic.smax";
> >> +            op = "smax";
> >>              break;
> >>      case nir_intrinsic_ssbo_atomic_umax:
> >> -            name = "llvm.amdgcn.buffer.atomic.umax";
> >> +            op = "umax";
> >>              break;
> >>      case nir_intrinsic_ssbo_atomic_and:
> >> -            name = "llvm.amdgcn.buffer.atomic.and";
> >> +            op = "and";
> >>              break;
> >>      case nir_intrinsic_ssbo_atomic_or:
> >> -            name = "llvm.amdgcn.buffer.atomic.or";
> >> +            op = "or";
> >>              break;
> >>      case nir_intrinsic_ssbo_atomic_xor:
> >> -            name = "llvm.amdgcn.buffer.atomic.xor";
> >> +            op = "xor";
> >>              break;
> >>      case nir_intrinsic_ssbo_atomic_exchange:
> >> -            name = "llvm.amdgcn.buffer.atomic.swap";
> >> +            op = "swap";
> >>              break;
> >>      case nir_intrinsic_ssbo_atomic_comp_swap:
> >> -            name = "llvm.amdgcn.buffer.atomic.cmpswap";
> >> +            op = "cmpswap";
> >>              break;
> >>      default:
> >>              abort();
> >>      }
> >>
> >> +    if (HAVE_LLVM >= 0x900 &&
> >> +        instr->intrinsic != nir_intrinsic_ssbo_atomic_comp_swap) {
> >> +            snprintf(name, sizeof(name),
> >> +                         "llvm.amdgcn.buffer.atomic.%s.i32", op);
> > The indention here seems off, compared to the else-case... (tabs vs
> > spaces?)
> Will fix before pushing.
> >
> >> +    } else {
> >> +            snprintf(name, sizeof(name),
> >> +                     "llvm.amdgcn.buffer.atomic.%s", op);
> >> +    }
> >> +
> >>      return ac_build_intrinsic(&ctx->ac, name, ctx->ac.i32, params,
> >> arg_count, 0);
> >>   }
> >>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev