Re: [Mesa-dev] [Mesa-stable] [PATCH 1/1] ac/nir: use ordered float comparisons except for not equal

[Samuel Pitoiset]

On 02/24/2018 02:43 PM, Marek Olšák wrote:



On Feb 24, 2018 5:52 AM, "Timothy Arceri" > wrote:


Using this on its own I believe will cause CTS regressions, which is
what the other patches were about. Feel free to take on the feedback
and come up with a proper solution. I'm not really sure how to
progress this.


The patch is correct. If there are any regression, it's because NIR or 
GLSL use incorrect transformations.


Yes, I think so.



Marek




On 24/02/18 00:21, Samuel Pitoiset wrote:

Original patch from Timothy Arceri, I have just fixed the
not equal case locally.

This fixes one important rendering issue in Wolfenstein 2
(the cutscene transition issue).

RadeonSI uses the same ordered comparisons, so I guess that
what we should do as well.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104302

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104905

Cc: mailto:mesa-sta...@lists.freedesktop.org>>
Signed-off-by: Samuel Pitoiset mailto:samuel.pitoi...@gmail.com>>
---
   src/amd/common/ac_nir_to_llvm.c | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c
b/src/amd/common/ac_nir_to_llvm.c
index 687157..bc1d16d2a4 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -1801,16 +1801,16 @@ static void visit_alu(struct
ac_nir_context *ctx, const nir_alu_instr *instr)
                 result = emit_int_cmp(&ctx->ac, LLVMIntUGE,
src[0], src[1]);
                 break;
         case nir_op_feq:
-               result = emit_float_cmp(&ctx->ac, LLVMRealUEQ,
src[0], src[1]);
+               result = emit_float_cmp(&ctx->ac, LLVMRealOEQ,
src[0], src[1]);
                 break;
         case nir_op_fne:
                 result = emit_float_cmp(&ctx->ac, LLVMRealUNE,
src[0], src[1]);
                 break;
         case nir_op_flt:
-               result = emit_float_cmp(&ctx->ac, LLVMRealULT,
src[0], src[1]);
+               result = emit_float_cmp(&ctx->ac, LLVMRealOLT,
src[0], src[1]);
                 break;
         case nir_op_fge:
-               result = emit_float_cmp(&ctx->ac, LLVMRealUGE,
src[0], src[1]);
+               result = emit_float_cmp(&ctx->ac, LLVMRealOGE,
src[0], src[1]);
                 break;
         case nir_op_fabs:
                 result = emit_intrin_1f_param(&ctx->ac,
"llvm.fabs",

___
mesa-stable mailing list
mesa-sta...@lists.freedesktop.org

https://lists.freedesktop.org/mailman/listinfo/mesa-stable




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

Re: [Mesa-dev] [Mesa-stable] [PATCH 1/1] ac/nir: use ordered float comparisons except for not equal

[Marek Olšák]

On Feb 24, 2018 5:52 AM, "Timothy Arceri"  wrote:

Using this on its own I believe will cause CTS regressions, which is what
the other patches were about. Feel free to take on the feedback and come up
with a proper solution. I'm not really sure how to progress this.


The patch is correct. If there are any regression, it's because NIR or GLSL
use incorrect transformations.

Marek




On 24/02/18 00:21, Samuel Pitoiset wrote:

> Original patch from Timothy Arceri, I have just fixed the
> not equal case locally.
>
> This fixes one important rendering issue in Wolfenstein 2
> (the cutscene transition issue).
>
> RadeonSI uses the same ordered comparisons, so I guess that
> what we should do as well.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104302
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104905
> Cc: 
> Signed-off-by: Samuel Pitoiset 
> ---
>   src/amd/common/ac_nir_to_llvm.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/amd/common/ac_nir_to_llvm.c
> b/src/amd/common/ac_nir_to_llvm.c
> index 687157..bc1d16d2a4 100644
> --- a/src/amd/common/ac_nir_to_llvm.c
> +++ b/src/amd/common/ac_nir_to_llvm.c
> @@ -1801,16 +1801,16 @@ static void visit_alu(struct ac_nir_context *ctx,
> const nir_alu_instr *instr)
> result = emit_int_cmp(&ctx->ac, LLVMIntUGE, src[0],
> src[1]);
> break;
> case nir_op_feq:
> -   result = emit_float_cmp(&ctx->ac, LLVMRealUEQ, src[0],
> src[1]);
> +   result = emit_float_cmp(&ctx->ac, LLVMRealOEQ, src[0],
> src[1]);
> break;
> case nir_op_fne:
> result = emit_float_cmp(&ctx->ac, LLVMRealUNE, src[0],
> src[1]);
> break;
> case nir_op_flt:
> -   result = emit_float_cmp(&ctx->ac, LLVMRealULT, src[0],
> src[1]);
> +   result = emit_float_cmp(&ctx->ac, LLVMRealOLT, src[0],
> src[1]);
> break;
> case nir_op_fge:
> -   result = emit_float_cmp(&ctx->ac, LLVMRealUGE, src[0],
> src[1]);
> +   result = emit_float_cmp(&ctx->ac, LLVMRealOGE, src[0],
> src[1]);
> break;
> case nir_op_fabs:
> result = emit_intrin_1f_param(&ctx->ac, "llvm.fabs",
>
> ___
mesa-stable mailing list
mesa-sta...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [Mesa-stable] [PATCH 1/1] ac/nir: use ordered float comparisons except for not equal

[Marek Olšák]

Reviewed-by: Marek Olšák 

Marek

On Fri, Feb 23, 2018 at 2:21 PM, Samuel Pitoiset
 wrote:
> Original patch from Timothy Arceri, I have just fixed the
> not equal case locally.
>
> This fixes one important rendering issue in Wolfenstein 2
> (the cutscene transition issue).
>
> RadeonSI uses the same ordered comparisons, so I guess that
> what we should do as well.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104302
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104905
> Cc: 
> Signed-off-by: Samuel Pitoiset 
> ---
>  src/amd/common/ac_nir_to_llvm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
> index 687157..bc1d16d2a4 100644
> --- a/src/amd/common/ac_nir_to_llvm.c
> +++ b/src/amd/common/ac_nir_to_llvm.c
> @@ -1801,16 +1801,16 @@ static void visit_alu(struct ac_nir_context *ctx, 
> const nir_alu_instr *instr)
> result = emit_int_cmp(&ctx->ac, LLVMIntUGE, src[0], src[1]);
> break;
> case nir_op_feq:
> -   result = emit_float_cmp(&ctx->ac, LLVMRealUEQ, src[0], 
> src[1]);
> +   result = emit_float_cmp(&ctx->ac, LLVMRealOEQ, src[0], 
> src[1]);
> break;
> case nir_op_fne:
> result = emit_float_cmp(&ctx->ac, LLVMRealUNE, src[0], 
> src[1]);
> break;
> case nir_op_flt:
> -   result = emit_float_cmp(&ctx->ac, LLVMRealULT, src[0], 
> src[1]);
> +   result = emit_float_cmp(&ctx->ac, LLVMRealOLT, src[0], 
> src[1]);
> break;
> case nir_op_fge:
> -   result = emit_float_cmp(&ctx->ac, LLVMRealUGE, src[0], 
> src[1]);
> +   result = emit_float_cmp(&ctx->ac, LLVMRealOGE, src[0], 
> src[1]);
> break;
> case nir_op_fabs:
> result = emit_intrin_1f_param(&ctx->ac, "llvm.fabs",
> --
> 2.16.2
>
> ___
> mesa-stable mailing list
> mesa-sta...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev