[Mesa-dev] [PATCH] llvmpipe: use vpkswss when dst is signed

[Roland Scheidegger]

Reviewed-by: Roland Scheidegger <sroland at vmware.com>

Am 17.01.2016 um 21:55 schrieb Oded Gabbay:
> This patch fixes a bug when building a pack instruction.
> 
> For POWER (altivec), in case the destination is signed and the
> src width is 32, we need to use vpkswss. The original code used vpkuwus,
> which emits an unsigned result.
> 
> This fixes the following piglit tests on ppc64le:
> - spec at arb_color_buffer_float@gl_rgba8-drawpixels
> - shaders at glsl-fs-fogscale
> - spec at arb_timer_query@query gl_timestamp
Sure about that last one? I'd suspect that's just a spurious failure.

Roland

> 
> I've also corrected some coding style issues in the function.
> 
> Signed-off-by: Oded Gabbay <oded.gabbay at gmail.com>
> ---
>  src/gallium/auxiliary/gallivm/lp_bld_pack.c | 44 ++++++++++++++++-------------
>  1 file changed, 25 insertions(+), 19 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_pack.c b/src/gallium/auxiliary/gallivm/lp_bld_pack.c
> index cdf6d80..39a5c3a 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_pack.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_pack.c
> @@ -461,15 +461,15 @@ lp_build_pack2(struct gallivm_state *gallivm,
>     assert(src_type.length * 2 == dst_type.length);
>  
>     /* Check for special cases first */
> -   if((util_cpu_caps.has_sse2 || util_cpu_caps.has_altivec) &&
> -       src_type.width * src_type.length >= 128) {
> +   if ((util_cpu_caps.has_sse2 || util_cpu_caps.has_altivec) &&
> +        src_type.width * src_type.length >= 128) {
>        const char *intrinsic = NULL;
>        boolean swap_intrinsic_operands = FALSE;
>  
>        switch(src_type.width) {
>        case 32:
>           if (util_cpu_caps.has_sse2) {
> -           if(dst_type.sign) {
> +           if (dst_type.sign) {
>                intrinsic = "llvm.x86.sse2.packssdw.128";
>             }
>             else {
> @@ -477,34 +477,39 @@ lp_build_pack2(struct gallivm_state *gallivm,
>                   intrinsic = "llvm.x86.sse41.packusdw";
>                }
>             }
> -         } else if (util_cpu_caps.has_altivec) {
> +         }
> +         else if (util_cpu_caps.has_altivec) {
>              if (dst_type.sign) {
> -              intrinsic = "llvm.ppc.altivec.vpkswus";
> -           } else {
> -              intrinsic = "llvm.ppc.altivec.vpkuwus";
> -           }
> +               intrinsic = "llvm.ppc.altivec.vpkswss";
> +            }
> +            else {
> +               intrinsic = "llvm.ppc.altivec.vpkuwus";
> +            }
>  #ifdef PIPE_ARCH_LITTLE_ENDIAN
> -           swap_intrinsic_operands = TRUE;
> +            swap_intrinsic_operands = TRUE;
>  #endif
>           }
>           break;
>        case 16:
>           if (dst_type.sign) {
>              if (util_cpu_caps.has_sse2) {
> -              intrinsic = "llvm.x86.sse2.packsswb.128";
> -            } else if (util_cpu_caps.has_altivec) {
> -              intrinsic = "llvm.ppc.altivec.vpkshss";
> +               intrinsic = "llvm.x86.sse2.packsswb.128";
> +            }
> +            else if (util_cpu_caps.has_altivec) {
> +               intrinsic = "llvm.ppc.altivec.vpkshss";
>  #ifdef PIPE_ARCH_LITTLE_ENDIAN
> -              swap_intrinsic_operands = TRUE;
> +               swap_intrinsic_operands = TRUE;
>  #endif
>              }
> -         } else {
> +         }
> +         else {
>              if (util_cpu_caps.has_sse2) {
> -              intrinsic = "llvm.x86.sse2.packuswb.128";
> -            } else if (util_cpu_caps.has_altivec) {
> -	      intrinsic = "llvm.ppc.altivec.vpkshus";
> +               intrinsic = "llvm.x86.sse2.packuswb.128";
> +            }
> +            else if (util_cpu_caps.has_altivec) {
> +               intrinsic = "llvm.ppc.altivec.vpkshus";
>  #ifdef PIPE_ARCH_LITTLE_ENDIAN
> -              swap_intrinsic_operands = TRUE;
> +               swap_intrinsic_operands = TRUE;
>  #endif
>              }
>           }
> @@ -516,7 +521,8 @@ lp_build_pack2(struct gallivm_state *gallivm,
>              LLVMTypeRef intr_vec_type = lp_build_vec_type(gallivm, intr_type);
>              if (swap_intrinsic_operands) {
>                 res = lp_build_intrinsic_binary(builder, intrinsic, intr_vec_type, hi, lo);
> -            } else {
> +            }
> +            else {
>                 res = lp_build_intrinsic_binary(builder, intrinsic, intr_vec_type, lo, hi);
>              }
>              if (dst_vec_type != intr_vec_type) {
>