[Mesa-dev] [PATCH 1/3] llvmpipe: don't store eo as 64bit int

[Brian Paul]

Looks OK to me.

For the series, Reviewed-by: Brian Paul <brianp at vmware.com>


On 01/02/2016 01:39 PM, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
>
> eo, just like dcdx and dcdy, cannot overflow 32bit.
> Store it as unsigned though just in case (it cannot be negative, but
> in theory twice as big as dcdx or dcdy so this gives it one more bit).
> This doesn't really change anything, albeit it might help minimally on
> 32bit archs.
> ---
>   src/gallium/drivers/llvmpipe/lp_rast.h         |  2 +-
>   src/gallium/drivers/llvmpipe/lp_rast_tri_tmp.h |  4 ++--
>   src/gallium/drivers/llvmpipe/lp_setup.c        |  5 +++++
>   src/gallium/drivers/llvmpipe/lp_setup_tri.c    | 16 ++++++++--------
>   4 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/src/gallium/drivers/llvmpipe/lp_rast.h b/src/gallium/drivers/llvmpipe/lp_rast.h
> index c19f931..db45cbb 100644
> --- a/src/gallium/drivers/llvmpipe/lp_rast.h
> +++ b/src/gallium/drivers/llvmpipe/lp_rast.h
> @@ -115,7 +115,7 @@ struct lp_rast_plane {
>      int32_t dcdy;
>
>      /* one-pixel sized trivial reject offsets for each plane */
> -   int64_t eo;
> +   uint32_t eo;
>   };
>
>   /**
> diff --git a/src/gallium/drivers/llvmpipe/lp_rast_tri_tmp.h b/src/gallium/drivers/llvmpipe/lp_rast_tri_tmp.h
> index 52f6e99..e0aea94 100644
> --- a/src/gallium/drivers/llvmpipe/lp_rast_tri_tmp.h
> +++ b/src/gallium/drivers/llvmpipe/lp_rast_tri_tmp.h
> @@ -82,7 +82,7 @@ TAG(do_block_16)(struct lp_rasterizer_task *task,
>         const int64_t dcdx = -IMUL64(plane[j].dcdx, 4);
>         const int64_t dcdy = IMUL64(plane[j].dcdy, 4);
>         const int64_t cox = IMUL64(plane[j].eo, 4);
> -      const int64_t ei = plane[j].dcdy - plane[j].dcdx - plane[j].eo;
> +      const int64_t ei = plane[j].dcdy - plane[j].dcdx - (int64_t)plane[j].eo;
>         const int64_t cio = IMUL64(ei, 4) - 1;
>
>         BUILD_MASKS(c[j] + cox,
> @@ -182,7 +182,7 @@ TAG(lp_rast_triangle)(struct lp_rasterizer_task *task,
>            const int64_t dcdx = -IMUL64(plane[j].dcdx, 16);
>            const int64_t dcdy = IMUL64(plane[j].dcdy, 16);
>            const int64_t cox = IMUL64(plane[j].eo, 16);
> -         const int64_t ei = plane[j].dcdy - plane[j].dcdx - plane[j].eo;
> +         const int64_t ei = plane[j].dcdy - plane[j].dcdx - (int64_t)plane[j].eo;
>            const int64_t cio = IMUL64(ei, 16) - 1;
>
>            BUILD_MASKS(c[j] + cox,
> diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c
> index ddbb88e..bd85051 100644
> --- a/src/gallium/drivers/llvmpipe/lp_setup.c
> +++ b/src/gallium/drivers/llvmpipe/lp_setup.c
> @@ -486,6 +486,11 @@ lp_setup_try_clear_zs(struct lp_setup_context *setup,
>                                      depth,
>                                      stencil);
>
> +   /*
> +    * XXX: should make a full mask here for things like D24X8,
> +    * otherwise we'll do a read-modify-write clear later which
> +    * should be unnecessary.
> +    */
>      zsmask = util_pack64_mask_z_stencil(setup->fb.zsbuf->format,
>                                          zmask32,
>                                          smask8);
> diff --git a/src/gallium/drivers/llvmpipe/lp_setup_tri.c b/src/gallium/drivers/llvmpipe/lp_setup_tri.c
> index 0c40fb3..98973de 100644
> --- a/src/gallium/drivers/llvmpipe/lp_setup_tri.c
> +++ b/src/gallium/drivers/llvmpipe/lp_setup_tri.c
> @@ -521,19 +521,19 @@ do_triangle_ccw(struct lp_setup_context *setup,
>      }
>
>      if (0) {
> -      debug_printf("p0: %"PRIx64"/%08x/%08x/%"PRIx64"\n",
> +      debug_printf("p0: %"PRIx64"/%08x/%08x/%08x\n",
>                      plane[0].c,
>                      plane[0].dcdx,
>                      plane[0].dcdy,
>                      plane[0].eo);
> -
> -      debug_printf("p1: %"PRIx64"/%08x/%08x/%"PRIx64"\n",
> +
> +      debug_printf("p1: %"PRIx64"/%08x/%08x/%08x\n",
>                      plane[1].c,
>                      plane[1].dcdx,
>                      plane[1].dcdy,
>                      plane[1].eo);
> -
> -      debug_printf("p2: %"PRIx64"/%08x/%08x/%"PRIx64"\n",
> +
> +      debug_printf("p2: %"PRIx64"/%08x/%08x/%08x\n",
>                      plane[2].c,
>                      plane[2].dcdx,
>                      plane[2].dcdy,
> @@ -594,7 +594,7 @@ do_triangle_ccw(struct lp_setup_context *setup,
>   static inline uint32_t
>   floor_pot(uint32_t n)
>   {
> -#if defined(PIPE_CC_GCC) && defined(PIPE_ARCH_X86)
> +#if defined(PIPE_CC_GCC) && (defined(PIPE_ARCH_X86) || defined(PIPE_ARCH_X86_64))
>      if (n == 0)
>         return 0;
>
> @@ -742,9 +742,9 @@ lp_setup_bin_triangle( struct lp_setup_context *setup,
>
>            ei[i] = (plane[i].dcdy -
>                     plane[i].dcdx -
> -                  plane[i].eo) << TILE_ORDER;
> +                  (int64_t)plane[i].eo) << TILE_ORDER;
>
> -         eo[i] = plane[i].eo << TILE_ORDER;
> +         eo[i] = (int64_t)plane[i].eo << TILE_ORDER;
>            xstep[i] = -(((int64_t)plane[i].dcdx) << TILE_ORDER);
>            ystep[i] = ((int64_t)plane[i].dcdy) << TILE_ORDER;
>         }
>