[Mesa-dev] [PATCH] gallivm: Improve lp_build_rcp_refine.

[Roland Scheidegger]

Looks good to me, albeit it's potentially minimally slower, so I'm
wondering if the higher precision is actually useful?
I guess though the last two steps could use lp_build_fmuladd?
Reviewed-by: Roland Scheidegger <sroland at vmware.com>



Am 25.06.19 um 11:17 schrieb Jose Fonseca:
> Use the alternative more accurate expression from
> https://en.wikipedia.org/wiki/Division_algorithm#Newton%E2%80%93Raphson_division
> 
> Tested by enabling this code path, and running gloss mesa demo.
> ---
>  src/gallium/auxiliary/gallivm/lp_bld_arit.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
> index 02fb81afe51..8aa5931eb69 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_arit.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
> @@ -2707,11 +2707,11 @@ lp_build_sqrt(struct lp_build_context *bld,
>  /**
>   * Do one Newton-Raphson step to improve reciprocate precision:
>   *
> - *   x_{i+1} = x_i * (2 - a * x_i)
> + *   x_{i+1} = x_i + x_i * (1 - a * x_i)
>   *
>   * XXX: Unfortunately this won't give IEEE-754 conformant results for 0 or
>   * +/-Inf, giving NaN instead.  Certain applications rely on this behavior,
> - * such as Google Earth, which does RCP(RSQRT(0.0) when drawing the Earth's
> + * such as Google Earth, which does RCP(RSQRT(0.0)) when drawing the Earth's
>   * halo. It would be necessary to clamp the argument to prevent this.
>   *
>   * See also:
> @@ -2724,12 +2724,13 @@ lp_build_rcp_refine(struct lp_build_context *bld,
>                      LLVMValueRef rcp_a)
>  {
>     LLVMBuilderRef builder = bld->gallivm->builder;
> -   LLVMValueRef two = lp_build_const_vec(bld->gallivm, bld->type, 2.0);
>     LLVMValueRef res;
>  
>     res = LLVMBuildFMul(builder, a, rcp_a, "");
> -   res = LLVMBuildFSub(builder, two, res, "");
> +   res = LLVMBuildFSub(builder, bld->one, res, "");
> +
>     res = LLVMBuildFMul(builder, rcp_a, res, "");
> +   res = LLVMBuildFAdd(builder, rcp_a, res, "");
>  
>     return res;
>  }
>