[Mesa-dev] [PATCH 05/20] radeonsi: implement shared atomics

[Bas Nieuwenhuizen]

On Wed, Apr 6, 2016 at 1:42 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 02.04.2016 08:10, Bas Nieuwenhuizen wrote:
>>
>> Signed-off-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
>> ---
>>   src/gallium/drivers/radeonsi/si_shader.c | 89
>> +++++++++++++++++++++++++++++++-
>>   1 file changed, 88 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_shader.c
>> b/src/gallium/drivers/radeonsi/si_shader.c
>> index 97d4404..7c7e9e5 100644
>> --- a/src/gallium/drivers/radeonsi/si_shader.c
>> +++ b/src/gallium/drivers/radeonsi/si_shader.c
>> @@ -3450,7 +3450,7 @@ static void atomic_fetch_args(
>>
>>                 buffer_append_args(ctx, emit_data, rsrc,
>> bld_base->uint_bld.zero,
>>                                    offset, true);
>> -       } else {
>> +       } else if (inst->Src[0].Register.File == TGSI_FILE_IMAGE) {
>>                 unsigned target = inst->Memory.Texture;
>>                 LLVMValueRef coords;
>>
>> @@ -3471,17 +3471,104 @@ static void atomic_fetch_args(
>>         }
>>   }
>>
>> +static void atomic_emit_memory(struct si_shader_context *ctx,
>> +                               struct lp_build_emit_data *emit_data) {
>> +       struct gallivm_state *gallivm = &ctx->radeon_bld.gallivm;
>> +       LLVMBuilderRef builder = gallivm->builder;
>> +       const struct tgsi_full_instruction * inst = emit_data->inst;
>> +       LLVMValueRef base, offset, result, arg;
>> +       int addr_space;
>> +
>> +       assert(!inst->Src[0].Register.Indirect);
>> +       base = ctx->memory_regions[inst->Src[0].Register.Index];
>> +       addr_space = LLVMGetPointerAddressSpace(LLVMTypeOf(base));
>> +
>> +       offset = lp_build_emit_fetch(&ctx->radeon_bld.soa.bld_base, inst,
>> 1, 0);
>> +
>> +       offset = LLVMBuildBitCast(builder, offset,
>> +
>> ctx->radeon_bld.soa.bld_base.uint_bld.elem_type, "");
>> +
>> +       base = LLVMBuildGEP(builder, base, &offset, 1, "");
>> +
>> +       base = LLVMBuildBitCast(builder, base,
>> +                                    LLVMPointerType(ctx->i32,
>> addr_space), "");
>
>
> Perhaps you could reduce code duplication by merging the pointer arithmetic
> and casting for load/store/atomic.
>
>> +
>> +       arg = lp_build_emit_fetch(&ctx->radeon_bld.soa.bld_base, inst, 2,
>> 0);
>> +       arg = LLVMBuildBitCast(builder, arg, ctx->i32, "");
>> +
>> +       if (inst->Instruction.Opcode == TGSI_OPCODE_ATOMCAS) {
>> +               LLVMValueRef new_data;
>> +               new_data =
>> lp_build_emit_fetch(&ctx->radeon_bld.soa.bld_base,
>> +                                              inst, 3, 0);
>> +
>> +               new_data = LLVMBuildBitCast(builder, new_data, ctx->i32,
>> "");
>> +
>> +#if HAVE_LLVM >= 0x309
>> +               result = LLVMBuildAtomicCmpXchg(builder, base, arg,
>> new_data,
>> +
>> LLVMAtomicOrderingSequentiallyConsistent,
>> +
>> LLVMAtomicOrderingSequentiallyConsistent,
>> +                                      false);
>> +#endif
>
>
> All of ARB_compute_shader already needs to be guarded by HAVE_LLVM >=
> 0x0309, so this particular #if/#endif seems redundant.

The LLVMBuildAtomicCmpXchg function has been introduced for LLVM 3.9,
so if we just rely on it not being called we get compile errors.

- Bas

>> +
>> +               result = LLVMBuildExtractValue(builder, result, 0, "");
>> +       } else {
>> +               LLVMAtomicRMWBinOp op;
>> +
>> +               switch(inst->Instruction.Opcode) {
>> +                       case TGSI_OPCODE_ATOMUADD:
>> +                               op = LLVMAtomicRMWBinOpAdd;
>> +                               break;
>> +                       case TGSI_OPCODE_ATOMXCHG:
>> +                               op = LLVMAtomicRMWBinOpXchg;
>> +                               break;
>> +                       case TGSI_OPCODE_ATOMAND:
>> +                               op = LLVMAtomicRMWBinOpAnd;
>> +                               break;
>> +                       case TGSI_OPCODE_ATOMOR:
>> +                               op = LLVMAtomicRMWBinOpOr;
>> +                               break;
>> +                       case TGSI_OPCODE_ATOMXOR:
>> +                               op = LLVMAtomicRMWBinOpXor;
>> +                               break;
>> +                       case TGSI_OPCODE_ATOMUMIN:
>> +                               op = LLVMAtomicRMWBinOpUMin;
>> +                               break;
>> +                       case TGSI_OPCODE_ATOMUMAX:
>> +                               op = LLVMAtomicRMWBinOpUMax;
>> +                               break;
>> +                       case TGSI_OPCODE_ATOMIMIN:
>> +                               op = LLVMAtomicRMWBinOpMin;
>> +                               break;
>> +                       case TGSI_OPCODE_ATOMIMAX:
>> +                               op = LLVMAtomicRMWBinOpMax;
>> +                               break;
>> +                       default:
>> +                               assert(0 && "unknown atomic opcode");
>> +               }
>> +
>> +               result = LLVMBuildAtomicRMW(builder, op, base, arg,
>> +
>> LLVMAtomicOrderingSequentiallyConsistent,
>> +                                      false);
>> +       }
>> +       emit_data->output[emit_data->chan] = LLVMBuildBitCast(builder,
>> result, emit_data->dst_type, "");
>> +}
>> +
>>   static void atomic_emit(
>>                 const struct lp_build_tgsi_action *action,
>>                 struct lp_build_tgsi_context *bld_base,
>>                 struct lp_build_emit_data *emit_data)
>>   {
>> +       struct si_shader_context *ctx = si_shader_context(bld_base);
>>         struct gallivm_state *gallivm = bld_base->base.gallivm;
>>         LLVMBuilderRef builder = gallivm->builder;
>>         const struct tgsi_full_instruction * inst = emit_data->inst;
>>         char intrinsic_name[40];
>>         LLVMValueRef tmp;
>>
>> +       if (inst->Src[0].Register.File == TGSI_FILE_MEMORY) {
>> +               atomic_emit_memory(ctx, emit_data);
>> +               return;
>> +       }
>>         if (inst->Src[0].Register.File == TGSI_FILE_BUFFER ||
>
>
> Add a new line between } and if.
>
> Cheers,
> Nicolai
>
>
>>             inst->Memory.Texture == TGSI_TEXTURE_BUFFER) {
>>                 snprintf(intrinsic_name, sizeof(intrinsic_name),
>>
>