On Mon, 2019-04-08 at 12:39 +0200, Samuel Pitoiset 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 <[email protected]> > > > --- > > > 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); > > > + } > > > +
I'm not an expert on LLVM, but the code changes looks good to me, and the reasoning makes sense. So if you want: Reviewed-by: Erik Faye-Lund <[email protected]> _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
