[Mesa-dev] [PATCH 3/4] i965/fs_surface_builder: Mask signed integers after conversion

[Francisco Jerez]

Jason Ekstrand <jason at jlekstrand.net> writes:

> ---
>  src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp
> index 75734d2..4adffdd 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp
> @@ -717,6 +717,15 @@ namespace {
>                    bld.emit_minmax(offset(dst, bld, c), offset(dst, bld, c),
>                                    brw_imm_d(-(int)scale(widths[c] - s) - 1),
>                                    BRW_CONDITIONAL_GE);
> +
> +               /* Mask off all but the bits we actually want.  Otherwise, if
> +                * we pass a negative number into the hardware when it's
> +                * expecting something like UINT8, it will happily clamp it to
> +                * +255 for us.
> +                */
> +               if (is_signed && widths[c] < 32)
> +                  bld.AND(offset(dst, bld, c), offset(dst, bld, c),
> +                          brw_imm_d((1 << widths[c]) - 1));
>              }
>           }
>  
> @@ -787,6 +796,15 @@ namespace {
>                 /* Convert to integer. */
>                 bld.RNDE(offset(fdst, bld, c), offset(fdst, bld, c));
>                 bld.MOV(offset(dst, bld, c), offset(fdst, bld, c));
> +
> +               /* Mask off all but the bits we actually want.  Otherwise, if
> +                * we pass a negative number into the hardware when it's
> +                * expecting something like UINT8, it will happily clamp it to
> +                * +255 for us.
> +                */
> +               if (is_signed && widths[c] < 32)
> +                  bld.AND(offset(dst, bld, c), offset(dst, bld, c),
> +                          brw_imm_d((1 << widths[c]) - 1));

Can you use the scale() helper to compute the bitmask like the code
above does?  Otherwise looks good:

Reviewed-by: Francisco Jerez <currojerez at riseup.net>

Thanks!

>              }
>           }
>  
> -- 
> 2.5.0.400.gff86faf
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160404/21ddf85c/attachment-0001.sig>