Re: [Mesa-dev] [PATCH 2/2] ac/nir: Fix ordering of parameters for image atomic cmpswap intrinsics

2017-07-07 Thread Alex Smith 
Thanks. Yes, first patch is superseded.

FWIW, James filed a bug against the CTS to add OpAtomicCompareExchange
tests a couple of weeks ago:
https://github.com/KhronosGroup/VK-GL-CTS/issues/47

On 7 July 2017 at 00:00, Bas Nieuwenhuizen  wrote:
> Thanks! Pushed and cc'd it to stable.
>
> Not pushing the first patch as I assume that is superseded by Connors patches.
>
> On Fri, Jun 30, 2017 at 12:15 PM, Alex Smith
>  wrote:
>> The NIR parameters are ordered "compare, data", matching GLSL, but both
>> the image and buffer LLVM intrinsics take them the other way around.
>> This is already handled correctly for SSBO atomics.
>>
>> Signed-off-by: Alex Smith 
>> ---
>>  src/amd/common/ac_nir_to_llvm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/amd/common/ac_nir_to_llvm.c 
>> b/src/amd/common/ac_nir_to_llvm.c
>> index 6845df8..89b7103 100644
>> --- a/src/amd/common/ac_nir_to_llvm.c
>> +++ b/src/amd/common/ac_nir_to_llvm.c
>> @@ -3442,9 +3442,9 @@ static LLVMValueRef visit_image_atomic(struct 
>> nir_to_llvm_context *ctx,
>> abort();
>> }
>>
>> -   params[param_count++] = get_src(ctx, instr->src[2]);
>> if (instr->intrinsic == nir_intrinsic_image_atomic_comp_swap)
>> params[param_count++] = get_src(ctx, instr->src[3]);
>> +   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], DESC_BUFFER);
>> --
>> 2.9.4
>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] ac/nir: Fix ordering of parameters for image atomic cmpswap intrinsics

2017-07-06 Thread Bas Nieuwenhuizen 
Thanks! Pushed and cc'd it to stable.

Not pushing the first patch as I assume that is superseded by Connors patches.

On Fri, Jun 30, 2017 at 12:15 PM, Alex Smith
 wrote:
> The NIR parameters are ordered "compare, data", matching GLSL, but both
> the image and buffer LLVM intrinsics take them the other way around.
> This is already handled correctly for SSBO atomics.
>
> Signed-off-by: Alex Smith 
> ---
>  src/amd/common/ac_nir_to_llvm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
> index 6845df8..89b7103 100644
> --- a/src/amd/common/ac_nir_to_llvm.c
> +++ b/src/amd/common/ac_nir_to_llvm.c
> @@ -3442,9 +3442,9 @@ static LLVMValueRef visit_image_atomic(struct 
> nir_to_llvm_context *ctx,
> abort();
> }
>
> -   params[param_count++] = get_src(ctx, instr->src[2]);
> if (instr->intrinsic == nir_intrinsic_image_atomic_comp_swap)
> params[param_count++] = get_src(ctx, instr->src[3]);
> +   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], DESC_BUFFER);
> --
> 2.9.4
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] ac/nir: Fix ordering of parameters for image atomic cmpswap intrinsics

2017-06-30 Thread Alex Smith 
Just compare/swap, not all the image atomics. From a quick grep it looks
like the CTS doesn't have any tests for compare/swap, but does for the
other atomics.

Alex

On 30 June 2017 at 16:45, Jason Ekstrand  wrote:

> Does this mean that image atomics are broken and radv passes the CTS
> anyway?  If so, someone should fine a bug against the CTS so we can keep
> track of the coverage hole.
>
>
>
> On June 30, 2017 3:15:57 AM Alex Smith 
> wrote:
>
> The NIR parameters are ordered "compare, data", matching GLSL, but both
>> the image and buffer LLVM intrinsics take them the other way around.
>> This is already handled correctly for SSBO atomics.
>>
>> Signed-off-by: Alex Smith 
>> ---
>>  src/amd/common/ac_nir_to_llvm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/amd/common/ac_nir_to_llvm.c
>> b/src/amd/common/ac_nir_to_llvm.c
>> index 6845df8..89b7103 100644
>> --- a/src/amd/common/ac_nir_to_llvm.c
>> +++ b/src/amd/common/ac_nir_to_llvm.c
>> @@ -3442,9 +3442,9 @@ static LLVMValueRef visit_image_atomic(struct
>> nir_to_llvm_context *ctx,
>> abort();
>> }
>>
>> -   params[param_count++] = get_src(ctx, instr->src[2]);
>> if (instr->intrinsic == nir_intrinsic_image_atomic_comp_swap)
>> params[param_count++] = get_src(ctx, instr->src[3]);
>> +   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],  DESC_BUFFER);
>> --
>> 2.9.4
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] ac/nir: Fix ordering of parameters for image atomic cmpswap intrinsics

2017-06-30 Thread Jason Ekstrand 
Does this mean that image atomics are broken and radv passes the CTS 
anyway?  If so, someone should fine a bug against the CTS so we can keep 
track of the coverage hole.



On June 30, 2017 3:15:57 AM Alex Smith  wrote:


The NIR parameters are ordered "compare, data", matching GLSL, but both
the image and buffer LLVM intrinsics take them the other way around.
This is already handled correctly for SSBO atomics.

Signed-off-by: Alex Smith 
---
 src/amd/common/ac_nir_to_llvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 6845df8..89b7103 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -3442,9 +3442,9 @@ static LLVMValueRef visit_image_atomic(struct 
nir_to_llvm_context *ctx,

abort();
}

-   params[param_count++] = get_src(ctx, instr->src[2]);
if (instr->intrinsic == nir_intrinsic_image_atomic_comp_swap)
params[param_count++] = get_src(ctx, instr->src[3]);
+   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], 
 DESC_BUFFER);

--
2.9.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 2/2] ac/nir: Fix ordering of parameters for image atomic cmpswap intrinsics

2017-06-30 Thread Alex Smith 
The NIR parameters are ordered "compare, data", matching GLSL, but both
the image and buffer LLVM intrinsics take them the other way around.
This is already handled correctly for SSBO atomics.

Signed-off-by: Alex Smith 
---
 src/amd/common/ac_nir_to_llvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index 6845df8..89b7103 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -3442,9 +3442,9 @@ static LLVMValueRef visit_image_atomic(struct 
nir_to_llvm_context *ctx,
abort();
}
 
-   params[param_count++] = get_src(ctx, instr->src[2]);
if (instr->intrinsic == nir_intrinsic_image_atomic_comp_swap)
params[param_count++] = get_src(ctx, instr->src[3]);
+   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], DESC_BUFFER);
-- 
2.9.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev