<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
This change is </div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
  Reviewed-by: Jose Fonseca <jfonseca@vmware.com></div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Regarding follow up change, do you think the LLVM pattern is sane/doable?</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
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 "<i>llvm.mulhi.*"  set of instrinsics</i>, as narrow pattern matching is bound to produce performance cliffs nobody
 will notice.</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<i><br>
</i></div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<i>Jose</i></div>
<div>
<div id="appendonsend"></div>
<div style="font-family:Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<br>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> sroland@vmware.com <sroland@vmware.com><br>
<b>Sent:</b> Wednesday, August 28, 2019 20:37<br>
<b>To:</b> mesa-dev@lists.freedesktop.org <mesa-dev@lists.freedesktop.org>; Jose Fonseca <jfonseca@vmware.com>; airlied@freedesktop.org <airlied@freedesktop.org><br>
<b>Cc:</b> Roland Scheidegger <sroland@vmware.com><br>
<b>Subject:</b> [PATCH] gallivm: use fallback code for mul_hi with llvm >= 7.0</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="PlainText">From: Roland Scheidegger <sroland@vmware.com><br>
<br>
LLVM 7.0 ditched the pmulu intrinsics.<br>
This is only a trivial patch to use the fallback code instead.<br>
It'll likely produce atrocious code since the pattern doesn't match what<br>
llvm itself uses in its autoupgrade paths, hence the pattern won't be<br>
recognized.<br>
<br>
Should fix <a href="https://bugs.freedesktop.org/show_bug.cgi?id=111496">https://bugs.freedesktop.org/show_bug.cgi?id=111496</a><br>
---<br>
 src/gallium/auxiliary/gallivm/lp_bld_arit.c | 7 ++++++-<br>
 1 file changed, 6 insertions(+), 1 deletion(-)<br>
<br>
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_arit.c b/src/gallium/auxiliary/gallivm/lp_bld_arit.c<br>
index c4931c0b230..f1866c6625f 100644<br>
--- a/src/gallium/auxiliary/gallivm/lp_bld_arit.c<br>
+++ b/src/gallium/auxiliary/gallivm/lp_bld_arit.c<br>
@@ -1169,8 +1169,13 @@ lp_build_mul_32_lohi_cpu(struct lp_build_context *bld,<br>
     * <a href="https://llvm.org/bugs/show_bug.cgi?id=30845">https://llvm.org/bugs/show_bug.cgi?id=30845</a><br>
     * So, whip up our own code, albeit only for length 4 and 8 (which<br>
     * should be good enough)...<br>
+    * FIXME: For llvm >= 7.0 we should match the autoupgrade pattern<br>
+    * (bitcast/and/mul/shuffle for unsigned, bitcast/shl/ashr/mul/shuffle<br>
+    * for signed), which the fallback code does not, without this llvm<br>
+    * will likely still produce atrocious code.<br>
     */<br>
-   if ((bld->type.length == 4 || bld->type.length == 8) &&<br>
+   if (HAVE_LLVM < 0x0700 &&<br>
+       (bld->type.length == 4 || bld->type.length == 8) &&<br>
        ((util_cpu_caps.has_sse2 && (bld->type.sign == 0)) ||<br>
         util_cpu_caps.has_sse4_1)) {<br>
       const char *intrinsic = NULL;<br>
-- <br>
2.17.1<br>
<br>
</div>
</span></font></div>
</div>
</body>
</html>