This change is

  Reviewed-by: Jose Fonseca <jfons...@vmware.com>

Regarding follow up change, do you think the LLVM pattern is sane/doable?

If not we should try ask them to reconsider relying strictly upon pattern 
matching.  I get the feeling upstream LLVM is throwing the baby with the water 
with these changes.  I do understand the advantages of moving away from vendor 
specific intrinsics, but I think that for things which have no natural 
representation on LLVM base IR, they should add a vendor-agnostic intrinsic, 
for example a new "llvm.mulhi.*"  set of instrinsics, as narrow pattern 
matching is bound to produce performance cliffs nobody will notice.

Jose

________________________________
From: srol...@vmware.com <srol...@vmware.com>
Sent: Wednesday, August 28, 2019 20:37
To: mesa-dev@lists.freedesktop.org <mesa-dev@lists.freedesktop.org>; Jose 
Fonseca <jfons...@vmware.com>; airl...@freedesktop.org <airl...@freedesktop.org>
Cc: Roland Scheidegger <srol...@vmware.com>
Subject: [PATCH] gallivm: use fallback code for mul_hi with llvm >= 7.0

From: Roland Scheidegger <srol...@vmware.com>

LLVM 7.0 ditched the pmulu intrinsics.
This is only a trivial patch to use the fallback code instead.
It'll likely produce atrocious code since the pattern doesn't match what
llvm itself uses in its autoupgrade paths, hence the pattern won't be
recognized.

Should fix https://bugs.freedesktop.org/show_bug.cgi?id=111496
---
 src/gallium/auxiliary/gallivm/lp_bld_arit.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c 
b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
index c4931c0b230..f1866c6625f 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_arit.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.c
@@ -1169,8 +1169,13 @@ lp_build_mul_32_lohi_cpu(struct lp_build_context *bld,
     * https://llvm.org/bugs/show_bug.cgi?id=30845
     * So, whip up our own code, albeit only for length 4 and 8 (which
     * should be good enough)...
+    * FIXME: For llvm >= 7.0 we should match the autoupgrade pattern
+    * (bitcast/and/mul/shuffle for unsigned, bitcast/shl/ashr/mul/shuffle
+    * for signed), which the fallback code does not, without this llvm
+    * will likely still produce atrocious code.
     */
-   if ((bld->type.length == 4 || bld->type.length == 8) &&
+   if (HAVE_LLVM < 0x0700 &&
+       (bld->type.length == 4 || bld->type.length == 8) &&
        ((util_cpu_caps.has_sse2 && (bld->type.sign == 0)) ||
         util_cpu_caps.has_sse4_1)) {
       const char *intrinsic = NULL;
--
2.17.1

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to