[llvm] r367891 - [InstCombine] combine mul+shl separated by zext
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 16 16:37:57 PDT 2019
I think that would work, but we should improve imul decomposition as
requested anyway in:
https://bugs.llvm.org/show_bug.cgi?id=42874
...and if we do that, I'm not sure that this transform really bought us
anything. I've reverted
with r369174, so we're not breaking anyone while investigating the way
forward.
On Fri, Aug 16, 2019 at 7:15 PM Roman Lebedev <lebedev.ri at gmail.com> wrote:
> Would an undo fold in backend be sufficient?
>
> On Sat, Aug 17, 2019 at 2:14 AM Sanjay Patel via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> >
> > Thanks, Eli.
> > I'll revert. We may want to limit the existing/simpler conversion to
> multiply as well (maybe using the datalayout?).
> >
> > On Fri, Aug 16, 2019 at 7:07 PM Eli Friedman <efriedma at quicinc.com>
> wrote:
> >>
> >> Hi Sanjay,
> >>
> >> 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.
> >>
> >> 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.
> >>
> >> -Eli
> >>
> >> -----Original Message-----
> >> From: llvm-commits <llvm-commits-bounces at lists.llvm.org> On Behalf Of
> Sanjay Patel via llvm-commits
> >> Sent: Monday, August 5, 2019 10:00 AM
> >> To: llvm-commits at lists.llvm.org
> >> Subject: [EXT] [llvm] r367891 - [InstCombine] combine mul+shl separated
> by zext
> >>
> >> Author: spatel
> >> Date: Mon Aug 5 09:59:58 2019
> >> New Revision: 367891
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=367891&view=rev
> >> Log:
> >> [InstCombine] combine mul+shl separated by zext
> >>
> >> This appears to slightly help patterns similar to what's
> >> shown in PR42874:
> >> https://bugs.llvm.org/show_bug.cgi?id=42874
> >> ...but not in the way requested.
> >>
> >> That fix will require some later IR and/or backend pass to
> >> decompose multiply/shifts into something more optimal per
> >> target. Those transforms already exist in some basic forms,
> >> but probably need enhancing to catch more cases.
> >>
> >> https://rise4fun.com/Alive/Qzv2
> >>
> >> Modified:
> >> llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp
> >> llvm/trunk/test/Transforms/InstCombine/shift.ll
> >>
> >> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp
> >> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp?rev=367891&r1=367890&r2=367891&view=diff
> >>
> ==============================================================================
> >> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp
> (original)
> >> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineShifts.cpp Mon
> Aug 5 09:59:58 2019
> >> @@ -715,14 +715,25 @@ Instruction *InstCombiner::visitShl(Bina
> >> unsigned ShAmt = ShAmtAPInt->getZExtValue();
> >> unsigned BitWidth = Ty->getScalarSizeInBits();
> >>
> >> - // shl (zext X), ShAmt --> zext (shl X, ShAmt)
> >> - // This is only valid if X would have zeros shifted out.
> >> Value *X;
> >> if (match(Op0, m_OneUse(m_ZExt(m_Value(X))))) {
> >> unsigned SrcWidth = X->getType()->getScalarSizeInBits();
> >> + // shl (zext X), ShAmt --> zext (shl X, ShAmt)
> >> + // This is only valid if X would have zeros shifted out.
> >> if (ShAmt < SrcWidth &&
> >> MaskedValueIsZero(X, APInt::getHighBitsSet(SrcWidth, ShAmt),
> 0, &I))
> >> return new ZExtInst(Builder.CreateShl(X, ShAmt), Ty);
> >> +
> >> + // shl (zext (mul MulOp, C2)), ShAmt --> mul (zext MulOp), (C2
> << ShAmt)
> >> + // This is valid if the high bits of the wider multiply are
> shifted out.
> >> + Value *MulOp;
> >> + const APInt *C2;
> >> + if (ShAmt >= (BitWidth - SrcWidth) &&
> >> + match(X, m_Mul(m_Value(MulOp), m_APInt(C2)))) {
> >> + Value *Zext = Builder.CreateZExt(MulOp, Ty);
> >> + Constant *NewMulC = ConstantInt::get(Ty,
> C2->zext(BitWidth).shl(ShAmt));
> >> + return BinaryOperator::CreateMul(Zext, NewMulC);
> >> + }
> >> }
> >>
> >> // (X >> C) << C --> X & (-1 << C)
> >>
> >> Modified: llvm/trunk/test/Transforms/InstCombine/shift.ll
> >> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/shift.ll?rev=367891&r1=367890&r2=367891&view=diff
> >>
> ==============================================================================
> >> --- llvm/trunk/test/Transforms/InstCombine/shift.ll (original)
> >> +++ llvm/trunk/test/Transforms/InstCombine/shift.ll Mon Aug 5 09:59:58
> 2019
> >> @@ -1223,9 +1223,8 @@ define <2 x i64> @shl_zext_splat_vec(<2
> >>
> >> define i64 @shl_zext_mul(i32 %t) {
> >> ; CHECK-LABEL: @shl_zext_mul(
> >> -; CHECK-NEXT: [[MUL:%.*]] = mul i32 [[T:%.*]], 16777215
> >> -; CHECK-NEXT: [[EXT:%.*]] = zext i32 [[MUL]] to i64
> >> -; CHECK-NEXT: [[SHL:%.*]] = shl nuw i64 [[EXT]], 32
> >> +; CHECK-NEXT: [[TMP1:%.*]] = zext i32 [[T:%.*]] to i64
> >> +; CHECK-NEXT: [[SHL:%.*]] = mul i64 [[TMP1]], 72057589742960640
> >> ; CHECK-NEXT: ret i64 [[SHL]]
> >> ;
> >> %mul = mul i32 %t, 16777215
> >> @@ -1236,9 +1235,8 @@ define i64 @shl_zext_mul(i32 %t) {
> >>
> >> define <3 x i17> @shl_zext_mul_splat(<3 x i5> %t) {
> >> ; CHECK-LABEL: @shl_zext_mul_splat(
> >> -; CHECK-NEXT: [[MUL:%.*]] = mul <3 x i5> [[T:%.*]], <i5 13, i5 13,
> i5 13>
> >> -; CHECK-NEXT: [[EXT:%.*]] = zext <3 x i5> [[MUL]] to <3 x i17>
> >> -; CHECK-NEXT: [[SHL:%.*]] = shl nuw <3 x i17> [[EXT]], <i17 12, i17
> 12, i17 12>
> >> +; CHECK-NEXT: [[TMP1:%.*]] = zext <3 x i5> [[T:%.*]] to <3 x i17>
> >> +; CHECK-NEXT: [[SHL:%.*]] = mul <3 x i17> [[TMP1]], <i17 53248, i17
> 53248, i17 53248>
> >> ; CHECK-NEXT: ret <3 x i17> [[SHL]]
> >> ;
> >> %mul = mul <3 x i5> %t, <i5 13, i5 13, i5 13>
> >> @@ -1281,8 +1279,8 @@ define i64 @shl_zext_mul_extra_use2(i32
> >> ; CHECK-LABEL: @shl_zext_mul_extra_use2(
> >> ; CHECK-NEXT: [[MUL:%.*]] = mul i32 [[T:%.*]], 16777215
> >> ; CHECK-NEXT: call void @use_i32(i32 [[MUL]])
> >> -; CHECK-NEXT: [[EXT:%.*]] = zext i32 [[MUL]] to i64
> >> -; CHECK-NEXT: [[SHL:%.*]] = shl nuw i64 [[EXT]], 32
> >> +; CHECK-NEXT: [[TMP1:%.*]] = zext i32 [[T]] to i64
> >> +; CHECK-NEXT: [[SHL:%.*]] = mul i64 [[TMP1]], 72057589742960640
> >> ; CHECK-NEXT: ret i64 [[SHL]]
> >> ;
> >> %mul = mul i32 %t, 16777215
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190816/536c8f8d/attachment.html>
More information about the llvm-commits
mailing list