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

[Oded Gabbay]

On Sun, Jan 17, 2016 at 11:16 PM, Roland Scheidegger <sroland at vmware.com> wrote:
> 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
>
Of course, removed that last line from the comment.

Oded

>>
>> 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) {
>>
>