[Mesa-dev] [PATCH] nir/search: Use the correct bit size for integer comparisons

[Iago Toral]

Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

On Thu, 2017-01-19 at 21:47 -0800, Jason Ekstrand wrote:
> The previous code always compared integers as 64-bit.  Due to
> variations
> in sign-extension in the code generated by nir_opt_algebraic.py, this
> meant that nir_search doesn't always do what you want.  Instead, 32-
> bit
> values should be matched as 32-bit and 64-bit values should be
> matched
> as 64-bit.  While we're here we unify the unsigned and signed paths.
> Now that we're using the right bit size, they should be the same
> since
> the only difference we had before was sign extension.
> 
> This gets the UE4 bitfield_extract optimization working again.  It
> had
> stopped working due to the constant 0xff00ff00 getting sign-extended
> when it shouldn't have.
> 
> Cc: "17.0 13.0" <mesa-stable at lists.freedesktop.org>
> Cc: Eric Anholt <eric at anholt.net>
> ---
>  src/compiler/nir/nir_search.c | 48 +++++++++++++++----------------
> ------------
>  1 file changed, 16 insertions(+), 32 deletions(-)
> 
> diff --git a/src/compiler/nir/nir_search.c
> b/src/compiler/nir/nir_search.c
> index 68275e4..dec56fe 100644
> --- a/src/compiler/nir/nir_search.c
> +++ b/src/compiler/nir/nir_search.c
> @@ -210,43 +210,27 @@ match_value(const nir_search_value *value,
> nir_alu_instr *instr, unsigned src,
>           return true;
>  
>        case nir_type_int:
> -         for (unsigned i = 0; i < num_components; ++i) {
> -            int64_t val;
> -            switch (load->def.bit_size) {
> -            case 32:
> -               val = load->value.i32[new_swizzle[i]];
> -               break;
> -            case 64:
> -               val = load->value.i64[new_swizzle[i]];
> -               break;
> -            default:
> -               unreachable("unknown bit size");
> -            }
> -
> -            if (val != const_val->data.i)
> -               return false;
> -         }
> -         return true;
> -
>        case nir_type_uint:
>        case nir_type_bool32:
> -         for (unsigned i = 0; i < num_components; ++i) {
> -            uint64_t val;
> -            switch (load->def.bit_size) {
> -            case 32:
> -               val = load->value.u32[new_swizzle[i]];
> -               break;
> -            case 64:
> -               val = load->value.u64[new_swizzle[i]];
> -               break;
> -            default:
> -               unreachable("unknown bit size");
> +         switch (load->def.bit_size) {
> +         case 32:
> +            for (unsigned i = 0; i < num_components; ++i) {
> +               if (load->value.u32[new_swizzle[i]] !=
> +                   (uint32_t)const_val->data.u)
> +                  return false;
>              }
> +            return true;
>  
> -            if (val != const_val->data.u)
> -               return false;
> +         case 64:
> +            for (unsigned i = 0; i < num_components; ++i) {
> +               if (load->value.u64[new_swizzle[i]] != const_val-
> >data.u)
> +                  return false;
> +            }
> +            return true;
> +
> +         default:
> +            unreachable("unknown bit size");
>           }
> -         return true;
>  
>        default:
>           unreachable("Invalid alu source type");