On 17/01/16 20:55, Oded Gabbay wrote:
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
I've also corrected some coding style issues in the function.
The if statement spacing chance is good.
But please don't split the "} else" in two lines: I know it's not the
Mesa standard, but it's the standard we use at VMware, and I also use at
Apitrace, so my brain just spews them out without thinking (given that
otherwise the formatting is quite similar.)
Given there's little hope to keep else statement and other things
consistent, I rather we just leave them alone as they are (be it in one
or two lines) to avoid any potential merge conflicts when cherry picking
changes etc.
Thank you regardless.
Jose
Signed-off-by: Oded Gabbay <[email protected]>
---
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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
