[Mesa-dev] [PATCH 4/8] i965/fs: Fix nir_op_fsign of absolute value.

[Ian Romanick]

On 01/24/2017 03:26 PM, Francisco Jerez wrote:
> This does point at the front-end emitting silly code that could have
> been optimized out, but the current fsign implementation would emit
> bogus IR if abs was set for the argument (because it would apply the
> abs modifier on an unsigned integer type), and we shouldn't rely on
> the upper layer's optimization passes for correctness.

Other than the atan2 code you emit later in the series, is there a test
for this?

> ---
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index e1ab598..e0c2fa0 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -701,7 +701,14 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)
>        break;
>  
>     case nir_op_fsign: {
> -      if (type_sz(op[0].type) < 8) {
> +      if (op[0].abs) {
> +         /* Straightforward since the source can be assumed to be
> +          * non-negative.
> +          */
> +         set_condmod(BRW_CONDITIONAL_NZ, bld.MOV(result, op[0]));
> +         set_predicate(BRW_PREDICATE_NORMAL, bld.MOV(result, brw_imm_f(1.0f)));

Does this work for DF source?

If we had an optimization pass for this, it would probably map
fsign(abs(a)) to float(a != 0) or double(a != 0).  This is different
from what we would generate for that, but I don't know which is better.

> +
> +      } else if (type_sz(op[0].type) < 8) {
>           /* AND(val, 0x80000000) gives the sign bit.
>            *
>            * Predicated OR ORs 1.0 (0x3f800000) with the sign bit if val is not
>