[Mesa-dev] [PATCH] nir: remove the abs call in is_neg_power_of_two

[Matt Turner]

On Mon, Feb 5, 2018 at 7:16 PM, Vlad Golovkin
<vlad.golovkin.mail at gmail.com> wrote:
> val->i32[swizzle[i]] is guaranteed to have non-positive value before the
> __is_power_of_two call, so unary minus is equivalent to abs in this case.
> ---
>  src/compiler/nir/nir_search_helpers.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/compiler/nir/nir_search_helpers.h b/src/compiler/nir/nir_search_helpers.h
> index 2e3bd137d6..66e1546ae6 100644
> --- a/src/compiler/nir/nir_search_helpers.h
> +++ b/src/compiler/nir/nir_search_helpers.h
> @@ -80,7 +80,7 @@ is_neg_power_of_two(nir_alu_instr *instr, unsigned src, unsigned num_components,
>        case nir_type_int:
>           if (val->i32[swizzle[i]] > 0)
>              return false;
> -         if (!__is_power_of_two(abs(val->i32[swizzle[i]])))
> +         if (!__is_power_of_two(-val->i32[swizzle[i]]))
>              return false;

I think your transformation is correct, but unnecessary and confusing.

You're right that val->i32[swizzle[i] must be 0 or negative.
__is_power_of_two() takes an unsigned value, so we need to remove the
sign bit, which can be done with a negation or an abs().

It takes more effort for me to understand why the negation is correct,
while the abs() is obvious.

Anyone else have a different opinion?