On Sun, Jan 17, 2016 at 11:16 PM, Roland Scheidegger <srol...@vmware.com> wrote:
> Reviewed-by: Roland Scheidegger <srol...@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@arb_color_buffer_float@gl_rgba8-drawpixels
>> - shaders@glsl-fs-fogscale
>> - spec@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.gab...@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) {
>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
