Re: [Mesa-dev] [PATCH] gallivm: Fix saturated signed psub/padd intrinsics on llvm 8

2019-10-17 Thread Jose Fonseca
Looks good.

Reviewed-by: Jose Fonseca 


From: srol...@vmware.com 
Sent: Thursday, October 17, 2019 03:20
To: Jose Fonseca ; airl...@freedesktop.org 
; mesa-dev@lists.freedesktop.org 

Cc: Roland Scheidegger ; mesa-sta...@lists.freedesktop.org 

Subject: [PATCH] gallivm: Fix saturated signed psub/padd intrinsics on llvm 8

From: Roland Scheidegger 

LLVM 8 did remove both the signed and unsigned sse2/avx intrinsics in
the end, and provide arch-independent llvm intrinsics instead.
Fixes a crash when using snorm framebuffers (tested with piglit
arb_color_buffer_float-render GL_RGBA8_SNORM -auto).

CC: 
---
 src/gallium/auxiliary/gallivm/lp_bld_arit.c | 28 -
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c 
b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
index 6b7ce9aacf9..53ee00e6767 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_arit.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
@@ -465,7 +465,7 @@ lp_build_add(struct lp_build_context *bld,
 return bld->one;

   if (!type.floating && !type.fixed) {
- if (LLVM_VERSION_MAJOR >= 9) {
+ if (LLVM_VERSION_MAJOR >= 8) {
 char intrin[32];
 intrinsic = type.sign ? "llvm.sadd.sat" : "llvm.uadd.sat";
 lp_format_intrinsic(intrin, sizeof intrin, intrinsic, 
bld->vec_type);
@@ -474,11 +474,9 @@ lp_build_add(struct lp_build_context *bld,
  if (type.width * type.length == 128) {
 if (util_cpu_caps.has_sse2) {
if (type.width == 8)
- intrinsic = type.sign ? "llvm.x86.sse2.padds.b" :
- LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.sse2.paddus.b" : NULL;
+ intrinsic = type.sign ? "llvm.x86.sse2.padds.b" : 
"llvm.x86.sse2.paddus.b";
if (type.width == 16)
- intrinsic = type.sign ? "llvm.x86.sse2.padds.w" :
- LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.sse2.paddus.w" : NULL;
+ intrinsic = type.sign ? "llvm.x86.sse2.padds.w" : 
"llvm.x86.sse2.paddus.w";
 } else if (util_cpu_caps.has_altivec) {
if (type.width == 8)
   intrinsic = type.sign ? "llvm.ppc.altivec.vaddsbs" : 
"llvm.ppc.altivec.vaddubs";
@@ -489,11 +487,9 @@ lp_build_add(struct lp_build_context *bld,
  if (type.width * type.length == 256) {
 if (util_cpu_caps.has_avx2) {
if (type.width == 8)
-  intrinsic = type.sign ? "llvm.x86.avx2.padds.b" :
-  LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.avx2.paddus.b" : NULL;
+  intrinsic = type.sign ? "llvm.x86.avx2.padds.b" : 
"llvm.x86.avx2.paddus.b";
if (type.width == 16)
-  intrinsic = type.sign ? "llvm.x86.avx2.padds.w" :
-  LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.avx2.paddus.w" : NULL;
+  intrinsic = type.sign ? "llvm.x86.avx2.padds.w" : 
"llvm.x86.avx2.paddus.w";
 }
  }
   }
@@ -793,7 +789,7 @@ lp_build_sub(struct lp_build_context *bld,
 return bld->zero;

   if (!type.floating && !type.fixed) {
- if (LLVM_VERSION_MAJOR >= 9) {
+ if (LLVM_VERSION_MAJOR >= 8) {
 char intrin[32];
 intrinsic = type.sign ? "llvm.ssub.sat" : "llvm.usub.sat";
 lp_format_intrinsic(intrin, sizeof intrin, intrinsic, 
bld->vec_type);
@@ -802,11 +798,9 @@ lp_build_sub(struct lp_build_context *bld,
  if (type.width * type.length == 128) {
 if (util_cpu_caps.has_sse2) {
if (type.width == 8)
-  intrinsic = type.sign ? "llvm.x86.sse2.psubs.b" :
-  LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.sse2.psubus.b" : NULL;
+  intrinsic = type.sign ? "llvm.x86.sse2.psubs.b" : 
"llvm.x86.sse2.psubus.b";
if (type.width == 16)
-  intrinsic = type.sign ? "llvm.x86.sse2.psubs.w" :
-  LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.sse2.psubus.w" : NULL;
+  intrinsic = type.sign ? "llvm.x86.sse2.psubs.w" : 
"llvm.x86.sse2.psubus.w";
 } else if (util_cpu_caps.has_altivec) {
if (type.width == 8)
   intrinsic = type.sign ? "llvm.ppc.altivec.vsubsbs" : 
"llvm.ppc.altivec.vsububs";
@@ -817,11 +811,9 @@ lp_build_sub(struct lp_build_context *bld,
  if (type.width * type.length == 256) {
 if (util_cpu_caps.has_avx2) {
if (type.width == 8)
-  intrinsic = type.sign ? "llvm.x86.avx2.psubs.b" :
-  LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.avx2.psubus.b" : NULL;
+  intrinsic = type.sign ? "llvm.x86.avx2.psubs.b" : 
"llvm.x86.avx2.psubus.b";

Re: [Mesa-dev] [PATCH] gallivm: Fix saturated signed psub/padd intrinsics on llvm 8

2019-10-16 Thread Dave Airlie
>
> From: Roland Scheidegger 

Thanks,

Reviewed-by: Dave Airlie 

>
> LLVM 8 did remove both the signed and unsigned sse2/avx intrinsics in
> the end, and provide arch-independent llvm intrinsics instead.
> Fixes a crash when using snorm framebuffers (tested with piglit
> arb_color_buffer_float-render GL_RGBA8_SNORM -auto).
>
> CC: 
> ---
>  src/gallium/auxiliary/gallivm/lp_bld_arit.c | 28 -
>  1 file changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c 
> b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
> index 6b7ce9aacf9..53ee00e6767 100644
> --- a/src/gallium/auxiliary/gallivm/lp_bld_arit.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
> @@ -465,7 +465,7 @@ lp_build_add(struct lp_build_context *bld,
>  return bld->one;
>
>if (!type.floating && !type.fixed) {
> - if (LLVM_VERSION_MAJOR >= 9) {
> + if (LLVM_VERSION_MAJOR >= 8) {
>  char intrin[32];
>  intrinsic = type.sign ? "llvm.sadd.sat" : "llvm.uadd.sat";
>  lp_format_intrinsic(intrin, sizeof intrin, intrinsic, 
> bld->vec_type);
> @@ -474,11 +474,9 @@ lp_build_add(struct lp_build_context *bld,
>   if (type.width * type.length == 128) {
>  if (util_cpu_caps.has_sse2) {
> if (type.width == 8)
> - intrinsic = type.sign ? "llvm.x86.sse2.padds.b" :
> - LLVM_VERSION_MAJOR < 8 ? 
> "llvm.x86.sse2.paddus.b" : NULL;
> + intrinsic = type.sign ? "llvm.x86.sse2.padds.b" : 
> "llvm.x86.sse2.paddus.b";
> if (type.width == 16)
> - intrinsic = type.sign ? "llvm.x86.sse2.padds.w" :
> - LLVM_VERSION_MAJOR < 8 ? 
> "llvm.x86.sse2.paddus.w" : NULL;
> + intrinsic = type.sign ? "llvm.x86.sse2.padds.w" : 
> "llvm.x86.sse2.paddus.w";
>  } else if (util_cpu_caps.has_altivec) {
> if (type.width == 8)
>intrinsic = type.sign ? "llvm.ppc.altivec.vaddsbs" : 
> "llvm.ppc.altivec.vaddubs";
> @@ -489,11 +487,9 @@ lp_build_add(struct lp_build_context *bld,
>   if (type.width * type.length == 256) {
>  if (util_cpu_caps.has_avx2) {
> if (type.width == 8)
> -  intrinsic = type.sign ? "llvm.x86.avx2.padds.b" :
> -  LLVM_VERSION_MAJOR < 8 ? 
> "llvm.x86.avx2.paddus.b" : NULL;
> +  intrinsic = type.sign ? "llvm.x86.avx2.padds.b" : 
> "llvm.x86.avx2.paddus.b";
> if (type.width == 16)
> -  intrinsic = type.sign ? "llvm.x86.avx2.padds.w" :
> -  LLVM_VERSION_MAJOR < 8 ? 
> "llvm.x86.avx2.paddus.w" : NULL;
> +  intrinsic = type.sign ? "llvm.x86.avx2.padds.w" : 
> "llvm.x86.avx2.paddus.w";
>  }
>   }
>}
> @@ -793,7 +789,7 @@ lp_build_sub(struct lp_build_context *bld,
>  return bld->zero;
>
>if (!type.floating && !type.fixed) {
> - if (LLVM_VERSION_MAJOR >= 9) {
> + if (LLVM_VERSION_MAJOR >= 8) {
>  char intrin[32];
>  intrinsic = type.sign ? "llvm.ssub.sat" : "llvm.usub.sat";
>  lp_format_intrinsic(intrin, sizeof intrin, intrinsic, 
> bld->vec_type);
> @@ -802,11 +798,9 @@ lp_build_sub(struct lp_build_context *bld,
>   if (type.width * type.length == 128) {
>  if (util_cpu_caps.has_sse2) {
> if (type.width == 8)
> -  intrinsic = type.sign ? "llvm.x86.sse2.psubs.b" :
> -  LLVM_VERSION_MAJOR < 8 ? 
> "llvm.x86.sse2.psubus.b" : NULL;
> +  intrinsic = type.sign ? "llvm.x86.sse2.psubs.b" : 
> "llvm.x86.sse2.psubus.b";
> if (type.width == 16)
> -  intrinsic = type.sign ? "llvm.x86.sse2.psubs.w" :
> -  LLVM_VERSION_MAJOR < 8 ? 
> "llvm.x86.sse2.psubus.w" : NULL;
> +  intrinsic = type.sign ? "llvm.x86.sse2.psubs.w" : 
> "llvm.x86.sse2.psubus.w";
>  } else if (util_cpu_caps.has_altivec) {
> if (type.width == 8)
>intrinsic = type.sign ? "llvm.ppc.altivec.vsubsbs" : 
> "llvm.ppc.altivec.vsububs";
> @@ -817,11 +811,9 @@ lp_build_sub(struct lp_build_context *bld,
>   if (type.width * type.length == 256) {
>  if (util_cpu_caps.has_avx2) {
> if (type.width == 8)
> -  intrinsic = type.sign ? "llvm.x86.avx2.psubs.b" :
> -  LLVM_VERSION_MAJOR < 8 ? 
> "llvm.x86.avx2.psubus.b" : NULL;
> +  intrinsic = type.sign ? "llvm.x86.avx2.psubs.b" : 
> "llvm.x86.avx2.psubus.b";
> if (type.width == 16)
> -  intrinsic = type.sign ? "llvm.x86.avx2.psubs.w" :
> -  

[Mesa-dev] [PATCH] gallivm: Fix saturated signed psub/padd intrinsics on llvm 8

2019-10-16 Thread sroland
From: Roland Scheidegger 

LLVM 8 did remove both the signed and unsigned sse2/avx intrinsics in
the end, and provide arch-independent llvm intrinsics instead.
Fixes a crash when using snorm framebuffers (tested with piglit
arb_color_buffer_float-render GL_RGBA8_SNORM -auto).

CC: 
---
 src/gallium/auxiliary/gallivm/lp_bld_arit.c | 28 -
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c 
b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
index 6b7ce9aacf9..53ee00e6767 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_arit.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
@@ -465,7 +465,7 @@ lp_build_add(struct lp_build_context *bld,
 return bld->one;
 
   if (!type.floating && !type.fixed) {
- if (LLVM_VERSION_MAJOR >= 9) {
+ if (LLVM_VERSION_MAJOR >= 8) {
 char intrin[32];
 intrinsic = type.sign ? "llvm.sadd.sat" : "llvm.uadd.sat";
 lp_format_intrinsic(intrin, sizeof intrin, intrinsic, 
bld->vec_type);
@@ -474,11 +474,9 @@ lp_build_add(struct lp_build_context *bld,
  if (type.width * type.length == 128) {
 if (util_cpu_caps.has_sse2) {
if (type.width == 8)
- intrinsic = type.sign ? "llvm.x86.sse2.padds.b" :
- LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.sse2.paddus.b" : NULL;
+ intrinsic = type.sign ? "llvm.x86.sse2.padds.b" : 
"llvm.x86.sse2.paddus.b";
if (type.width == 16)
- intrinsic = type.sign ? "llvm.x86.sse2.padds.w" :
- LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.sse2.paddus.w" : NULL;
+ intrinsic = type.sign ? "llvm.x86.sse2.padds.w" : 
"llvm.x86.sse2.paddus.w";
 } else if (util_cpu_caps.has_altivec) {
if (type.width == 8)
   intrinsic = type.sign ? "llvm.ppc.altivec.vaddsbs" : 
"llvm.ppc.altivec.vaddubs";
@@ -489,11 +487,9 @@ lp_build_add(struct lp_build_context *bld,
  if (type.width * type.length == 256) {
 if (util_cpu_caps.has_avx2) {
if (type.width == 8)
-  intrinsic = type.sign ? "llvm.x86.avx2.padds.b" :
-  LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.avx2.paddus.b" : NULL;
+  intrinsic = type.sign ? "llvm.x86.avx2.padds.b" : 
"llvm.x86.avx2.paddus.b";
if (type.width == 16)
-  intrinsic = type.sign ? "llvm.x86.avx2.padds.w" :
-  LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.avx2.paddus.w" : NULL;
+  intrinsic = type.sign ? "llvm.x86.avx2.padds.w" : 
"llvm.x86.avx2.paddus.w";
 }
  }
   }
@@ -793,7 +789,7 @@ lp_build_sub(struct lp_build_context *bld,
 return bld->zero;
 
   if (!type.floating && !type.fixed) {
- if (LLVM_VERSION_MAJOR >= 9) {
+ if (LLVM_VERSION_MAJOR >= 8) {
 char intrin[32];
 intrinsic = type.sign ? "llvm.ssub.sat" : "llvm.usub.sat";
 lp_format_intrinsic(intrin, sizeof intrin, intrinsic, 
bld->vec_type);
@@ -802,11 +798,9 @@ lp_build_sub(struct lp_build_context *bld,
  if (type.width * type.length == 128) {
 if (util_cpu_caps.has_sse2) {
if (type.width == 8)
-  intrinsic = type.sign ? "llvm.x86.sse2.psubs.b" :
-  LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.sse2.psubus.b" : NULL;
+  intrinsic = type.sign ? "llvm.x86.sse2.psubs.b" : 
"llvm.x86.sse2.psubus.b";
if (type.width == 16)
-  intrinsic = type.sign ? "llvm.x86.sse2.psubs.w" :
-  LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.sse2.psubus.w" : NULL;
+  intrinsic = type.sign ? "llvm.x86.sse2.psubs.w" : 
"llvm.x86.sse2.psubus.w";
 } else if (util_cpu_caps.has_altivec) {
if (type.width == 8)
   intrinsic = type.sign ? "llvm.ppc.altivec.vsubsbs" : 
"llvm.ppc.altivec.vsububs";
@@ -817,11 +811,9 @@ lp_build_sub(struct lp_build_context *bld,
  if (type.width * type.length == 256) {
 if (util_cpu_caps.has_avx2) {
if (type.width == 8)
-  intrinsic = type.sign ? "llvm.x86.avx2.psubs.b" :
-  LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.avx2.psubus.b" : NULL;
+  intrinsic = type.sign ? "llvm.x86.avx2.psubs.b" : 
"llvm.x86.avx2.psubus.b";
if (type.width == 16)
-  intrinsic = type.sign ? "llvm.x86.avx2.psubs.w" :
-  LLVM_VERSION_MAJOR < 8 ? 
"llvm.x86.avx2.psubus.w" : NULL;
+  intrinsic = type.sign ? "llvm.x86.avx2.psubs.w" : 
"llvm.x86.avx2.psubus.w";
 }
  }
   }
-- 
2.17.1