<div dir="ltr"><div>Thanks, Eli.</div><div>I'll revert. We may want to limit the existing/simpler conversion to multiply as well (maybe using the datalayout?).<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Aug 16, 2019 at 7:07 PM Eli Friedman <<a href="mailto:efriedma@quicinc.com">efriedma@quicinc.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Sanjay,<br>
<br>
This appears to be causing a link failure in one of my team's internal tests: an i32 multiply followed by an i128 shift gets transformed into an i128 mul, and the mul is lowered to a call to __multi3. __multi3 generally doesn't exist on 32-bit ARM.<br>
<br>
Granted, __multi3 is a silly way to lower the multiply, but I'm not sure how much instcombine should depend on the backend being "smart" here. The clang frontend and SROA currently assume arbitrary shifts can always be lowered somehow, but making the same assumption about multiplies seem riskier.<br>
<br>
-Eli<br>
<br>
-----Original Message-----<br>
From: llvm-commits <<a href="mailto:llvm-commits-bounces@lists.llvm.org" target="_blank">llvm-commits-bounces@lists.llvm.org</a>> On Behalf Of Sanjay Patel via llvm-commits<br>
Sent: Monday, August 5, 2019 10:00 AM<br>
To: <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
Subject: [EXT] [llvm] r367891 - [InstCombine] combine mul+shl separated by zext<br>
<br>
Author: spatel<br>
Date: Mon Aug 5 09:59:58 2019<br>
New Revision: 367891<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=367891&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=367891&view=rev</a><br>
Log:<br>
[InstCombine] combine mul+shl separated by zext<br>
<br>
This appears to slightly help patterns similar to what's<br>
shown in PR42874:<br>
<a href="https://bugs.llvm.org/show_bug.cgi?id=42874" rel="noreferrer" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=42874</a><br>
...but not in the way requested.<br>
<br>
That fix will require some later IR and/or backend pass to<br>
decompose multiply/shifts into something more optimal per<br>
target. Those transforms already exist in some basic forms,<br>
but probably need enhancing to catch more cases.<br>
<br>
<a href="https://rise4fun.com/Alive/Qzv2" rel="noreferrer" target="_blank">https://rise4fun.com/Alive/Qzv2</a><br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp<br>
llvm/trunk/test/Transforms/InstCombine/shift.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp?rev=367891&r1=367890&r2=367891&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp?rev=367891&r1=367890&r2=367891&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp Mon Aug 5 09:59:58 2019<br>
@@ -715,14 +715,25 @@ Instruction *InstCombiner::visitShl(Bina<br>
unsigned ShAmt = ShAmtAPInt->getZExtValue();<br>
unsigned BitWidth = Ty->getScalarSizeInBits();<br>
<br>
- // shl (zext X), ShAmt --> zext (shl X, ShAmt)<br>
- // This is only valid if X would have zeros shifted out.<br>
Value *X;<br>
if (match(Op0, m_OneUse(m_ZExt(m_Value(X))))) {<br>
unsigned SrcWidth = X->getType()->getScalarSizeInBits();<br>
+ // shl (zext X), ShAmt --> zext (shl X, ShAmt)<br>
+ // This is only valid if X would have zeros shifted out.<br>
if (ShAmt < SrcWidth &&<br>
MaskedValueIsZero(X, APInt::getHighBitsSet(SrcWidth, ShAmt), 0, &I))<br>
return new ZExtInst(Builder.CreateShl(X, ShAmt), Ty);<br>
+<br>
+ // shl (zext (mul MulOp, C2)), ShAmt --> mul (zext MulOp), (C2 << ShAmt)<br>
+ // This is valid if the high bits of the wider multiply are shifted out.<br>
+ Value *MulOp;<br>
+ const APInt *C2;<br>
+ if (ShAmt >= (BitWidth - SrcWidth) &&<br>
+ match(X, m_Mul(m_Value(MulOp), m_APInt(C2)))) {<br>
+ Value *Zext = Builder.CreateZExt(MulOp, Ty);<br>
+ Constant *NewMulC = ConstantInt::get(Ty, C2->zext(BitWidth).shl(ShAmt));<br>
+ return BinaryOperator::CreateMul(Zext, NewMulC);<br>
+ }<br>
}<br>
<br>
// (X >> C) << C --> X & (-1 << C)<br>
<br>
Modified: llvm/trunk/test/Transforms/InstCombine/shift.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/shift.ll?rev=367891&r1=367890&r2=367891&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/shift.ll?rev=367891&r1=367890&r2=367891&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/InstCombine/shift.ll (original)<br>
+++ llvm/trunk/test/Transforms/InstCombine/shift.ll Mon Aug 5 09:59:58 2019<br>
@@ -1223,9 +1223,8 @@ define <2 x i64> @shl_zext_splat_vec(<2<br>
<br>
define i64 @shl_zext_mul(i32 %t) {<br>
; CHECK-LABEL: @shl_zext_mul(<br>
-; CHECK-NEXT: [[MUL:%.*]] = mul i32 [[T:%.*]], 16777215<br>
-; CHECK-NEXT: [[EXT:%.*]] = zext i32 [[MUL]] to i64<br>
-; CHECK-NEXT: [[SHL:%.*]] = shl nuw i64 [[EXT]], 32<br>
+; CHECK-NEXT: [[TMP1:%.*]] = zext i32 [[T:%.*]] to i64<br>
+; CHECK-NEXT: [[SHL:%.*]] = mul i64 [[TMP1]], 72057589742960640<br>
; CHECK-NEXT: ret i64 [[SHL]]<br>
;<br>
%mul = mul i32 %t, 16777215<br>
@@ -1236,9 +1235,8 @@ define i64 @shl_zext_mul(i32 %t) {<br>
<br>
define <3 x i17> @shl_zext_mul_splat(<3 x i5> %t) {<br>
; CHECK-LABEL: @shl_zext_mul_splat(<br>
-; CHECK-NEXT: [[MUL:%.*]] = mul <3 x i5> [[T:%.*]], <i5 13, i5 13, i5 13><br>
-; CHECK-NEXT: [[EXT:%.*]] = zext <3 x i5> [[MUL]] to <3 x i17><br>
-; CHECK-NEXT: [[SHL:%.*]] = shl nuw <3 x i17> [[EXT]], <i17 12, i17 12, i17 12><br>
+; CHECK-NEXT: [[TMP1:%.*]] = zext <3 x i5> [[T:%.*]] to <3 x i17><br>
+; CHECK-NEXT: [[SHL:%.*]] = mul <3 x i17> [[TMP1]], <i17 53248, i17 53248, i17 53248><br>
; CHECK-NEXT: ret <3 x i17> [[SHL]]<br>
;<br>
%mul = mul <3 x i5> %t, <i5 13, i5 13, i5 13><br>
@@ -1281,8 +1279,8 @@ define i64 @shl_zext_mul_extra_use2(i32<br>
; CHECK-LABEL: @shl_zext_mul_extra_use2(<br>
; CHECK-NEXT: [[MUL:%.*]] = mul i32 [[T:%.*]], 16777215<br>
; CHECK-NEXT: call void @use_i32(i32 [[MUL]])<br>
-; CHECK-NEXT: [[EXT:%.*]] = zext i32 [[MUL]] to i64<br>
-; CHECK-NEXT: [[SHL:%.*]] = shl nuw i64 [[EXT]], 32<br>
+; CHECK-NEXT: [[TMP1:%.*]] = zext i32 [[T]] to i64<br>
+; CHECK-NEXT: [[SHL:%.*]] = mul i64 [[TMP1]], 72057589742960640<br>
; CHECK-NEXT: ret i64 [[SHL]]<br>
;<br>
%mul = mul i32 %t, 16777215<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>