[llvm] 427520a - Revert "[InstCombine] factorize min/max intrinsic ops with common operand"
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 12 13:39:41 PDT 2021
It was a copy-paste error on my part - missed the null check for the common
operand, so we just crashed on any pattern with 4 unique min/max operands.
See the negative test here:
https://reviews.llvm.org/rG14eefa57f2b6
On Thu, Aug 12, 2021 at 3:42 PM Roman Lebedev via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Reproducer?
>
> On Thu, Aug 12, 2021 at 10:36 PM Amy Huang via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> >
> >
> > Author: Amy Huang
> > Date: 2021-08-12T12:36:25-07:00
> > New Revision: 427520a8fa09cc9e65e3ee118be7cc5f1ac21d10
> >
> > URL:
> https://github.com/llvm/llvm-project/commit/427520a8fa09cc9e65e3ee118be7cc5f1ac21d10
> > DIFF:
> https://github.com/llvm/llvm-project/commit/427520a8fa09cc9e65e3ee118be7cc5f1ac21d10.diff
> >
> > LOG: Revert "[InstCombine] factorize min/max intrinsic ops with common
> operand"
> >
> > This reverts commit 6de1dbbd09c12abbec7eb187ffa1afbd47302dfa because it
> causes a
> > compiler crash.
> >
> > Added:
> >
> >
> > Modified:
> > llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
> > llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
> >
> > Removed:
> >
> >
> >
> >
> ################################################################################
> > diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
> b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
> > index d6ccf0fcc708..210652e23377 100644
> > --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
> > +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
> > @@ -795,60 +795,6 @@ static Instruction
> *foldClampRangeOfTwo(IntrinsicInst *II,
> > return SelectInst::Create(Cmp, ConstantInt::get(II->getType(), *C0),
> I1);
> > }
> >
> > -/// Reduce a sequence of min/max intrinsics with a common operand.
> > -static Instruction *factorizeMinMaxTree(IntrinsicInst *II) {
> > - // Match 3 of the same min/max ops. Example: umin(umin(), umin()).
> > - auto *LHS = dyn_cast<IntrinsicInst>(II->getArgOperand(0));
> > - auto *RHS = dyn_cast<IntrinsicInst>(II->getArgOperand(1));
> > - Intrinsic::ID MinMaxID = II->getIntrinsicID();
> > - if (!LHS || !RHS || LHS->getIntrinsicID() != MinMaxID ||
> > - RHS->getIntrinsicID() != MinMaxID ||
> > - (!LHS->hasOneUse() && !RHS->hasOneUse()))
> > - return nullptr;
> > -
> > - Value *A = LHS->getArgOperand(0);
> > - Value *B = LHS->getArgOperand(1);
> > - Value *C = RHS->getArgOperand(0);
> > - Value *D = RHS->getArgOperand(1);
> > -
> > - // Look for a common operand.
> > - Value *MinMaxOp = nullptr;
> > - Value *ThirdOp = nullptr;
> > - if (LHS->hasOneUse()) {
> > - // If the LHS is only used in this chain and the RHS is used
> outside of it,
> > - // reuse the RHS min/max because that will eliminate the LHS.
> > - if (D == A || C == A) {
> > - // min(min(a, b), min(c, a)) --> min(min(c, a), b)
> > - // min(min(a, b), min(a, d)) --> min(min(a, d), b)
> > - MinMaxOp = RHS;
> > - ThirdOp = B;
> > - } else if (D == B || C == B) {
> > - // min(min(a, b), min(c, b)) --> min(min(c, b), a)
> > - // min(min(a, b), min(b, d)) --> min(min(b, d), a)
> > - MinMaxOp = RHS;
> > - ThirdOp = A;
> > - }
> > - } else {
> > - assert(RHS->hasOneUse() && "Expected one-use operand");
> > - // Reuse the LHS. This will eliminate the RHS.
> > - if (D == A || D == B) {
> > - // min(min(a, b), min(c, a)) --> min(min(a, b), c)
> > - // min(min(a, b), min(c, b)) --> min(min(a, b), c)
> > - MinMaxOp = LHS;
> > - ThirdOp = C;
> > - } else if (C == A || C == B) {
> > - // min(min(a, b), min(b, d)) --> min(min(a, b), d)
> > - // min(min(a, b), min(c, b)) --> min(min(a, b), d)
> > - MinMaxOp = LHS;
> > - ThirdOp = D;
> > - }
> > - }
> > -
> > - Module *Mod = II->getModule();
> > - Function *MinMax = Intrinsic::getDeclaration(Mod, MinMaxID,
> II->getType());
> > - return CallInst::Create(MinMax, { MinMaxOp, ThirdOp });
> > -}
> > -
> > /// CallInst simplification. This mostly only handles folding of
> intrinsic
> > /// instructions. For normal calls, it allows visitCallBase to do the
> heavy
> > /// lifting.
> > @@ -1110,9 +1056,6 @@ Instruction
> *InstCombinerImpl::visitCallInst(CallInst &CI) {
> > if (Instruction *R = FoldOpIntoSelect(*II, Sel))
> > return R;
> >
> > - if (Instruction *NewMinMax = factorizeMinMaxTree(II))
> > - return NewMinMax;
> > -
> > break;
> > }
> > case Intrinsic::bswap: {
> >
> > diff --git a/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
> b/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
> > index 8693526f198c..54e133ded3c9 100644
> > --- a/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
> > +++ b/llvm/test/Transforms/InstCombine/minmax-intrinsics.ll
> > @@ -907,8 +907,9 @@ define <3 x i1> @umin_ne_zero2(<3 x i8> %a, <3 x i8>
> %b) {
> >
> > define i8 @smax(i8 %x, i8 %y, i8 %z) {
> > ; CHECK-LABEL: @smax(
> > -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.smax.i8(i8 [[X:%.*]], i8
> [[Z:%.*]])
> > -; CHECK-NEXT: [[M3:%.*]] = call i8 @llvm.smax.i8(i8 [[M2]], i8
> [[Y:%.*]])
> > +; CHECK-NEXT: [[M1:%.*]] = call i8 @llvm.smax.i8(i8 [[X:%.*]], i8
> [[Y:%.*]])
> > +; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.smax.i8(i8 [[X]], i8
> [[Z:%.*]])
> > +; CHECK-NEXT: [[M3:%.*]] = call i8 @llvm.smax.i8(i8 [[M1]], i8
> [[M2]])
> > ; CHECK-NEXT: ret i8 [[M3]]
> > ;
> > %m1 = call i8 @llvm.smax.i8(i8 %x, i8 %y)
> > @@ -919,8 +920,9 @@ define i8 @smax(i8 %x, i8 %y, i8 %z) {
> >
> > define <3 x i8> @smin(<3 x i8> %x, <3 x i8> %y, <3 x i8> %z) {
> > ; CHECK-LABEL: @smin(
> > -; CHECK-NEXT: [[M2:%.*]] = call <3 x i8> @llvm.smin.v3i8(<3 x i8>
> [[X:%.*]], <3 x i8> [[Z:%.*]])
> > -; CHECK-NEXT: [[M3:%.*]] = call <3 x i8> @llvm.smin.v3i8(<3 x i8>
> [[M2]], <3 x i8> [[Y:%.*]])
> > +; CHECK-NEXT: [[M1:%.*]] = call <3 x i8> @llvm.smin.v3i8(<3 x i8>
> [[Y:%.*]], <3 x i8> [[X:%.*]])
> > +; CHECK-NEXT: [[M2:%.*]] = call <3 x i8> @llvm.smin.v3i8(<3 x i8>
> [[X]], <3 x i8> [[Z:%.*]])
> > +; CHECK-NEXT: [[M3:%.*]] = call <3 x i8> @llvm.smin.v3i8(<3 x i8>
> [[M1]], <3 x i8> [[M2]])
> > ; CHECK-NEXT: ret <3 x i8> [[M3]]
> > ;
> > %m1 = call <3 x i8> @llvm.smin.v3i8(<3 x i8> %y, <3 x i8> %x)
> > @@ -933,7 +935,8 @@ define i8 @umax(i8 %x, i8 %y, i8 %z) {
> > ; CHECK-LABEL: @umax(
> > ; CHECK-NEXT: [[M1:%.*]] = call i8 @llvm.umax.i8(i8 [[X:%.*]], i8
> [[Y:%.*]])
> > ; CHECK-NEXT: call void @use(i8 [[M1]])
> > -; CHECK-NEXT: [[M3:%.*]] = call i8 @llvm.umax.i8(i8 [[M1]], i8
> [[Z:%.*]])
> > +; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.umax.i8(i8 [[Z:%.*]], i8
> [[X]])
> > +; CHECK-NEXT: [[M3:%.*]] = call i8 @llvm.umax.i8(i8 [[M1]], i8
> [[M2]])
> > ; CHECK-NEXT: ret i8 [[M3]]
> > ;
> > %m1 = call i8 @llvm.umax.i8(i8 %x, i8 %y)
> > @@ -945,9 +948,10 @@ define i8 @umax(i8 %x, i8 %y, i8 %z) {
> >
> > define i8 @umin(i8 %x, i8 %y, i8 %z) {
> > ; CHECK-LABEL: @umin(
> > -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.umin.i8(i8 [[Z:%.*]], i8
> [[X:%.*]])
> > +; CHECK-NEXT: [[M1:%.*]] = call i8 @llvm.umin.i8(i8 [[Y:%.*]], i8
> [[X:%.*]])
> > +; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.umin.i8(i8 [[Z:%.*]], i8
> [[X]])
> > ; CHECK-NEXT: call void @use(i8 [[M2]])
> > -; CHECK-NEXT: [[M3:%.*]] = call i8 @llvm.umin.i8(i8 [[M2]], i8
> [[Y:%.*]])
> > +; CHECK-NEXT: [[M3:%.*]] = call i8 @llvm.umin.i8(i8 [[M1]], i8
> [[M2]])
> > ; CHECK-NEXT: ret i8 [[M3]]
> > ;
> > %m1 = call i8 @llvm.umin.i8(i8 %y, i8 %x)
> >
> >
> >
> > _______________________________________________
> > 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/20210812/57db1a82/attachment.html>
More information about the llvm-commits
mailing list