Re: [Mesa-dev] [PATCH 02/10] gallivm: implement the correct version of LRP

[Roland Scheidegger]

Am 15.10.2015 um 16:44 schrieb Marek Olšák:
> Any comment or is this okay with people? Given, "(1-t)*a + t*b", the
> original code didn't return b for t=1 because it's "floating-point".
> 
> Marek
> 
> On Sun, Oct 11, 2015 at 3:29 AM, Marek Olšák  wrote:
>> From: Marek Olšák 
>>
>> The previous version has precision issues. This can be a problem
>> with tessellation. Sadly, I can't find the article where I read it
>> anymore. I'm not sure if the unsafe-fp-math flag would be enough to revert
>> this.
>> ---
>>  src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c | 13 +++--
>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c 
>> b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
>> index 0ad78b0..512558b 100644
>> --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
>> @@ -538,12 +538,13 @@ lrp_emit(
>> struct lp_build_tgsi_context * bld_base,
>> struct lp_build_emit_data * emit_data)
>>  {
>> -   LLVMValueRef tmp;
>> -   tmp = lp_build_emit_llvm_binary(bld_base, TGSI_OPCODE_SUB,
>> -   emit_data->args[1],
>> -   emit_data->args[2]);
>> -   emit_data->output[emit_data->chan] = lp_build_emit_llvm_ternary(bld_base,
>> -TGSI_OPCODE_MAD, emit_data->args[0], tmp, 
>> emit_data->args[2]);
>> +   struct lp_build_context *bld = _base->base;
>> +   LLVMValueRef inv, a, b;
>> +
>> +   inv = lp_build_sub(bld, bld_base->base.one, emit_data->args[0]);
>> +   a = lp_build_mul(bld, emit_data->args[1], emit_data->args[0]);
>> +   b = lp_build_mul(bld, emit_data->args[2], inv);
>> +   emit_data->output[emit_data->chan] = lp_build_add(bld, a, b);
>>  }
>>
>>  /* TGSI_OPCODE_MAD */
>> --

Please add a comment why it's using t*a + (1-t)*b and not (a-b)*t + b.
Though it is yet another thing we should have some more control over in
tgsi. Because if you're willing to allow unsafe-fp-math, then you should
also be willing to accept the simpler formula (I'm quite sure
unsafe-fp-math would be allowed to turn one formula into the other).
But otherwise I guess this is ok - it is the formula specified by glsl
after all.

Roland

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

Re: [Mesa-dev] [PATCH 02/10] gallivm: implement the correct version of LRP

[Marek Olšák]

Any comment or is this okay with people? Given, "(1-t)*a + t*b", the
original code didn't return b for t=1 because it's "floating-point".

Marek

On Sun, Oct 11, 2015 at 3:29 AM, Marek Olšák  wrote:
> From: Marek Olšák 
>
> The previous version has precision issues. This can be a problem
> with tessellation. Sadly, I can't find the article where I read it
> anymore. I'm not sure if the unsafe-fp-math flag would be enough to revert
> this.
> ---
>  src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c | 13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c 
> b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
> index 0ad78b0..512558b 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
> @@ -538,12 +538,13 @@ lrp_emit(
> struct lp_build_tgsi_context * bld_base,
> struct lp_build_emit_data * emit_data)
>  {
> -   LLVMValueRef tmp;
> -   tmp = lp_build_emit_llvm_binary(bld_base, TGSI_OPCODE_SUB,
> -   emit_data->args[1],
> -   emit_data->args[2]);
> -   emit_data->output[emit_data->chan] = lp_build_emit_llvm_ternary(bld_base,
> -TGSI_OPCODE_MAD, emit_data->args[0], tmp, 
> emit_data->args[2]);
> +   struct lp_build_context *bld = _base->base;
> +   LLVMValueRef inv, a, b;
> +
> +   inv = lp_build_sub(bld, bld_base->base.one, emit_data->args[0]);
> +   a = lp_build_mul(bld, emit_data->args[1], emit_data->args[0]);
> +   b = lp_build_mul(bld, emit_data->args[2], inv);
> +   emit_data->output[emit_data->chan] = lp_build_add(bld, a, b);
>  }
>
>  /* TGSI_OPCODE_MAD */
> --
> 2.1.4
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 02/10] gallivm: implement the correct version of LRP

[Jose Fonseca]

Roland was on PTO.

IMO, the change makes sense from a numeric accuracy POV.

I fear this might cause some slowdown with llvmpipe (two muls intead of 
one), but hopefully it won't be significant.  The accuracy issue could 
cause glitches to llvmpipe too.


Jose

On 15/10/15 15:44, Marek Olšák wrote:

Any comment or is this okay with people? Given, "(1-t)*a + t*b", the
original code didn't return b for t=1 because it's "floating-point".

Marek

On Sun, Oct 11, 2015 at 3:29 AM, Marek Olšák  wrote:

From: Marek Olšák 

The previous version has precision issues. This can be a problem
with tessellation. Sadly, I can't find the article where I read it
anymore. I'm not sure if the unsafe-fp-math flag would be enough to revert
this.
---
  src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c | 13 +++--
  1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c 
b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
index 0ad78b0..512558b 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
@@ -538,12 +538,13 @@ lrp_emit(
 struct lp_build_tgsi_context * bld_base,
 struct lp_build_emit_data * emit_data)
  {
-   LLVMValueRef tmp;
-   tmp = lp_build_emit_llvm_binary(bld_base, TGSI_OPCODE_SUB,
-   emit_data->args[1],
-   emit_data->args[2]);
-   emit_data->output[emit_data->chan] = lp_build_emit_llvm_ternary(bld_base,
-TGSI_OPCODE_MAD, emit_data->args[0], tmp, 
emit_data->args[2]);
+   struct lp_build_context *bld = _base->base;
+   LLVMValueRef inv, a, b;
+
+   inv = lp_build_sub(bld, bld_base->base.one, emit_data->args[0]);
+   a = lp_build_mul(bld, emit_data->args[1], emit_data->args[0]);
+   b = lp_build_mul(bld, emit_data->args[2], inv);
+   emit_data->output[emit_data->chan] = lp_build_add(bld, a, b);
  }

  /* TGSI_OPCODE_MAD */
--
2.1.4


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



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

Re: [Mesa-dev] [PATCH 02/10] gallivm: implement the correct version of LRP

[Jose Fonseca]

On 15/10/15 16:20, Roland Scheidegger wrote:

Am 15.10.2015 um 16:44 schrieb Marek Olšák:

Any comment or is this okay with people? Given, "(1-t)*a + t*b", the
original code didn't return b for t=1 because it's "floating-point".

Marek

On Sun, Oct 11, 2015 at 3:29 AM, Marek Olšák  wrote:

From: Marek Olšák 

The previous version has precision issues. This can be a problem
with tessellation. Sadly, I can't find the article where I read it
anymore. I'm not sure if the unsafe-fp-math flag would be enough to revert
this.
---
  src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c | 13 +++--
  1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c 
b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
index 0ad78b0..512558b 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_action.c
@@ -538,12 +538,13 @@ lrp_emit(
 struct lp_build_tgsi_context * bld_base,
 struct lp_build_emit_data * emit_data)
  {
-   LLVMValueRef tmp;
-   tmp = lp_build_emit_llvm_binary(bld_base, TGSI_OPCODE_SUB,
-   emit_data->args[1],
-   emit_data->args[2]);
-   emit_data->output[emit_data->chan] = lp_build_emit_llvm_ternary(bld_base,
-TGSI_OPCODE_MAD, emit_data->args[0], tmp, 
emit_data->args[2]);
+   struct lp_build_context *bld = _base->base;
+   LLVMValueRef inv, a, b;
+
+   inv = lp_build_sub(bld, bld_base->base.one, emit_data->args[0]);
+   a = lp_build_mul(bld, emit_data->args[1], emit_data->args[0]);
+   b = lp_build_mul(bld, emit_data->args[2], inv);
+   emit_data->output[emit_data->chan] = lp_build_add(bld, a, b);
  }

  /* TGSI_OPCODE_MAD */
--


Please add a comment why it's using t*a + (1-t)*b and not (a-b)*t + b.
Though it is yet another thing we should have some more control over in
tgsi.


> Because if you're willing to allow unsafe-fp-math, then you should

also be willing to accept the simpler formula (I'm quite sure
unsafe-fp-math would be allowed to turn one formula into the other).


Yep, that's my understanding of "unsafe fp math" too.

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