<div dir="ltr"><div>Yes - undef makes higher-level reasoning difficult. We had an icmp-based simplification with: <br></div><div><a href="https://reviews.llvm.org/D84655" target="_blank">https://reviews.llvm.org/D84655</a></div><div><br></div><div>But we're making progress on stubbing out the danger with patches like:</div><div><span><a href="https://reviews.llvm.org/D84948" rel="noreferrer" target="_blank">https://reviews.llvm.org/D84948</a><br></span></div><div><a href="https://reviews.llvm.org/D84250">https://reviews.llvm.org/D84250</a></div><div><br><span></span></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 5, 2020 at 3:01 PM Nikita Popov <<a href="mailto:nikita.ppv@gmail.com" target="_blank">nikita.ppv@gmail.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"><div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 5, 2020 at 7:27 PM Philip Reames via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</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">A thought on a possible generalization.<br>
<br>
Instead of restricting this to constants, could we use generic reasoning <br>
about Value relations? If we implement a isKnownPredicate variant, we <br>
could handle cases such as the following:<br>
<br>
%b = add nsw %a, 1<br>
%c = smin(%a, %b) ;; %c == %b<br>
<br>
Weirdly, I can't find analogous code for selects. I'd have assumed we <br>
handled cases like this...<br>
<br>
Philip<br></blockquote><div><br></div><div>More general folds in this area are currently blocked on <a href="https://reviews.llvm.org/D84792" target="_blank">https://reviews.llvm.org/D84792</a>.<br></div><div><br></div><div>Nikita<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
On 8/4/20 5:44 AM, Sanjay Patel via llvm-commits wrote:<br>
> Author: Sanjay Patel<br>
> Date: 2020-08-04T08:44:48-04:00<br>
> New Revision: 04e45ae1c6d2fdbf3fd4242df69d1511df757d48<br>
><br>
> URL: <a href="https://github.com/llvm/llvm-project/commit/04e45ae1c6d2fdbf3fd4242df69d1511df757d48" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/04e45ae1c6d2fdbf3fd4242df69d1511df757d48</a><br>
> DIFF: <a href="https://github.com/llvm/llvm-project/commit/04e45ae1c6d2fdbf3fd4242df69d1511df757d48.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/04e45ae1c6d2fdbf3fd4242df69d1511df757d48.diff</a><br>
><br>
> LOG: [InstSimplify] fold nested min/max intrinsics with constant operands<br>
><br>
> This is based on the existing code for the non-intrinsic idioms<br>
> in InstCombine.<br>
><br>
> The vector constant constraint is non-obvious: undefs should be<br>
> ok in the outer call, but they can't propagate safely from the<br>
> inner call in all cases. Example:<br>
><br>
> <a href="https://alive2.llvm.org/ce/z/-2bVbM" rel="noreferrer" target="_blank">https://alive2.llvm.org/ce/z/-2bVbM</a><br>
> define <2 x i8> @src(<2 x i8> %x) {<br>
> %0:<br>
> %m = umin <2 x i8> %x, { 7, undef }<br>
> %m2 = umin <2 x i8> { 9, 9 }, %m<br>
> ret <2 x i8> %m2<br>
> }<br>
> =><br>
> define <2 x i8> @tgt(<2 x i8> %x) {<br>
> %0:<br>
> %m = umin <2 x i8> %x, { 7, undef }<br>
> ret <2 x i8> %m<br>
> }<br>
> Transformation doesn't verify!<br>
> ERROR: Value mismatch<br>
><br>
> Example:<br>
> <2 x i8> %x = < undef, undef ><br>
><br>
> Source:<br>
> <2 x i8> %m = < #x00 (0) [based on undef value], #x00 (0) ><br>
> <2 x i8> %m2 = < #x00 (0), #x00 (0) ><br>
><br>
> Target:<br>
> <2 x i8> %m = < #x07 (7), #x10 (16) ><br>
> Source value: < #x00 (0), #x00 (0) ><br>
> Target value: < #x07 (7), #x10 (16) ><br>
><br>
> Added:<br>
> <br>
><br>
> Modified:<br>
> llvm/lib/Analysis/InstructionSimplify.cpp<br>
> llvm/test/Transforms/InstSimplify/maxmin_intrinsics.ll<br>
><br>
> Removed:<br>
> <br>
><br>
><br>
> ################################################################################<br>
> diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp<br>
> index 2119ddcc7649..6b8f8e3acc17 100644<br>
> --- a/llvm/lib/Analysis/InstructionSimplify.cpp<br>
> +++ b/llvm/lib/Analysis/InstructionSimplify.cpp<br>
> @@ -5285,6 +5285,21 @@ static Value *simplifyBinaryIntrinsic(Function *F, Value *Op0, Value *Op1,<br>
> // umin(i8 %x, i8 255) --> %x<br>
> if (*C == getMaxMinLimit(getMaxMinOpposite(IID), BitWidth))<br>
> return Op0;<br>
> +<br>
> + // Remove nested call if constant operands allow it. Example:<br>
> + // max (max X, 7), 5 -> max X, 7<br>
> + auto *MinMax0 = dyn_cast<IntrinsicInst>(Op0);<br>
> + if (MinMax0 && MinMax0->getIntrinsicID() == IID) {<br>
> + // TODO: loosen undef/splat restrictions for vector constants.<br>
> + Value *M00 = MinMax0->getOperand(0), *M01 = MinMax0->getOperand(1);<br>
> + const APInt *InnerC;<br>
> + if ((match(M00, m_APInt(InnerC)) || match(M01, m_APInt(InnerC))) &&<br>
> + ((IID == Intrinsic::smax && InnerC->sge(*C)) ||<br>
> + (IID == Intrinsic::smin && InnerC->sle(*C)) ||<br>
> + (IID == Intrinsic::umax && InnerC->uge(*C)) ||<br>
> + (IID == Intrinsic::umin && InnerC->ule(*C))))<br>
> + return Op0;<br>
> + }<br>
> }<br>
> <br>
> break;<br>
><br>
> diff --git a/llvm/test/Transforms/InstSimplify/maxmin_intrinsics.ll b/llvm/test/Transforms/InstSimplify/maxmin_intrinsics.ll<br>
> index 3aa19e91e0e3..6b10853dd78f 100644<br>
> --- a/llvm/test/Transforms/InstSimplify/maxmin_intrinsics.ll<br>
> +++ b/llvm/test/Transforms/InstSimplify/maxmin_intrinsics.ll<br>
> @@ -627,8 +627,7 @@ define i8 @umin_smin(i8 %x, i8 %y) {<br>
> define i8 @umax_umax_constants(i8 %x) {<br>
> ; CHECK-LABEL: @umax_umax_constants(<br>
> ; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.umax.i8(i8 [[X:%.*]], i8 9)<br>
> -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.umax.i8(i8 7, i8 [[M]])<br>
> -; CHECK-NEXT: ret i8 [[M2]]<br>
> +; CHECK-NEXT: ret i8 [[M]]<br>
> ;<br>
> %m = call i8 @llvm.umax.i8(i8 %x, i8 9)<br>
> %m2 = call i8 @llvm.umax.i8(i8 7, i8 %m)<br>
> @@ -638,8 +637,7 @@ define i8 @umax_umax_constants(i8 %x) {<br>
> define i8 @umax_umax_constants_commute1(i8 %x) {<br>
> ; CHECK-LABEL: @umax_umax_constants_commute1(<br>
> ; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.umax.i8(i8 -128, i8 [[X:%.*]])<br>
> -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.umax.i8(i8 7, i8 [[M]])<br>
> -; CHECK-NEXT: ret i8 [[M2]]<br>
> +; CHECK-NEXT: ret i8 [[M]]<br>
> ;<br>
> %m = call i8 @llvm.umax.i8(i8 128, i8 %x)<br>
> %m2 = call i8 @llvm.umax.i8(i8 7, i8 %m)<br>
> @@ -649,8 +647,7 @@ define i8 @umax_umax_constants_commute1(i8 %x) {<br>
> define i8 @umax_umax_constants_commute2(i8 %x) {<br>
> ; CHECK-LABEL: @umax_umax_constants_commute2(<br>
> ; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.umax.i8(i8 [[X:%.*]], i8 -56)<br>
> -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.umax.i8(i8 [[M]], i8 127)<br>
> -; CHECK-NEXT: ret i8 [[M2]]<br>
> +; CHECK-NEXT: ret i8 [[M]]<br>
> ;<br>
> %m = call i8 @llvm.umax.i8(i8 %x, i8 200)<br>
> %m2 = call i8 @llvm.umax.i8(i8 %m, i8 127)<br>
> @@ -660,8 +657,7 @@ define i8 @umax_umax_constants_commute2(i8 %x) {<br>
> define <2 x i8> @umax_umax_constants_commute3(<2 x i8> %x) {<br>
> ; CHECK-LABEL: @umax_umax_constants_commute3(<br>
> ; CHECK-NEXT: [[M:%.*]] = call <2 x i8> @llvm.umax.v2i8(<2 x i8> <i8 -2, i8 -2>, <2 x i8> [[X:%.*]])<br>
> -; CHECK-NEXT: [[M2:%.*]] = call <2 x i8> @llvm.umax.v2i8(<2 x i8> [[M]], <2 x i8> <i8 -128, i8 -128>)<br>
> -; CHECK-NEXT: ret <2 x i8> [[M2]]<br>
> +; CHECK-NEXT: ret <2 x i8> [[M]]<br>
> ;<br>
> %m = call <2 x i8> @llvm.umax.v2i8(<2 x i8> <i8 254, i8 254>, <2 x i8> %x)<br>
> %m2 = call <2 x i8> @llvm.umax.v2i8(<2 x i8> %m, <2 x i8> <i8 128, i8 128>)<br>
> @@ -671,8 +667,7 @@ define <2 x i8> @umax_umax_constants_commute3(<2 x i8> %x) {<br>
> define i8 @umin_umin_constants(i8 %x) {<br>
> ; CHECK-LABEL: @umin_umin_constants(<br>
> ; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.umin.i8(i8 [[X:%.*]], i8 7)<br>
> -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.umin.i8(i8 9, i8 [[M]])<br>
> -; CHECK-NEXT: ret i8 [[M2]]<br>
> +; CHECK-NEXT: ret i8 [[M]]<br>
> ;<br>
> %m = call i8 @llvm.umin.i8(i8 %x, i8 7)<br>
> %m2 = call i8 @llvm.umin.i8(i8 9, i8 %m)<br>
> @@ -682,8 +677,7 @@ define i8 @umin_umin_constants(i8 %x) {<br>
> define i8 @umin_umin_constants_commute1(i8 %x) {<br>
> ; CHECK-LABEL: @umin_umin_constants_commute1(<br>
> ; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.umin.i8(i8 7, i8 [[X:%.*]])<br>
> -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.umin.i8(i8 -128, i8 [[M]])<br>
> -; CHECK-NEXT: ret i8 [[M2]]<br>
> +; CHECK-NEXT: ret i8 [[M]]<br>
> ;<br>
> %m = call i8 @llvm.umin.i8(i8 7, i8 %x)<br>
> %m2 = call i8 @llvm.umin.i8(i8 128, i8 %m)<br>
> @@ -693,8 +687,7 @@ define i8 @umin_umin_constants_commute1(i8 %x) {<br>
> define <2 x i8> @umin_umin_constants_commute2(<2 x i8> %x) {<br>
> ; CHECK-LABEL: @umin_umin_constants_commute2(<br>
> ; CHECK-NEXT: [[M:%.*]] = call <2 x i8> @llvm.umin.v2i8(<2 x i8> [[X:%.*]], <2 x i8> <i8 127, i8 127>)<br>
> -; CHECK-NEXT: [[M2:%.*]] = call <2 x i8> @llvm.umin.v2i8(<2 x i8> [[M]], <2 x i8> <i8 -56, i8 undef>)<br>
> -; CHECK-NEXT: ret <2 x i8> [[M2]]<br>
> +; CHECK-NEXT: ret <2 x i8> [[M]]<br>
> ;<br>
> %m = call <2 x i8> @llvm.umin.v2i8(<2 x i8> %x, <2 x i8> <i8 127, i8 127>)<br>
> %m2 = call <2 x i8> @llvm.umin.v2i8(<2 x i8> %m, <2 x i8> <i8 200, i8 undef>)<br>
> @@ -704,8 +697,7 @@ define <2 x i8> @umin_umin_constants_commute2(<2 x i8> %x) {<br>
> define i8 @umin_umin_constants_commute3(i8 %x) {<br>
> ; CHECK-LABEL: @umin_umin_constants_commute3(<br>
> ; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.umin.i8(i8 -128, i8 [[X:%.*]])<br>
> -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.umin.i8(i8 [[M]], i8 -2)<br>
> -; CHECK-NEXT: ret i8 [[M2]]<br>
> +; CHECK-NEXT: ret i8 [[M]]<br>
> ;<br>
> %m = call i8 @llvm.umin.i8(i8 128, i8 %x)<br>
> %m2 = call i8 @llvm.umin.i8(i8 %m, i8 254)<br>
> @@ -715,8 +707,7 @@ define i8 @umin_umin_constants_commute3(i8 %x) {<br>
> define i8 @smax_smax_constants(i8 %x) {<br>
> ; CHECK-LABEL: @smax_smax_constants(<br>
> ; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.smax.i8(i8 [[X:%.*]], i8 9)<br>
> -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.smax.i8(i8 7, i8 [[M]])<br>
> -; CHECK-NEXT: ret i8 [[M2]]<br>
> +; CHECK-NEXT: ret i8 [[M]]<br>
> ;<br>
> %m = call i8 @llvm.smax.i8(i8 %x, i8 9)<br>
> %m2 = call i8 @llvm.smax.i8(i8 7, i8 %m)<br>
> @@ -726,8 +717,7 @@ define i8 @smax_smax_constants(i8 %x) {<br>
> define <2 x i8> @smax_smax_constants_commute1(<2 x i8> %x) {<br>
> ; CHECK-LABEL: @smax_smax_constants_commute1(<br>
> ; CHECK-NEXT: [[M:%.*]] = call <2 x i8> @llvm.smax.v2i8(<2 x i8> <i8 7, i8 7>, <2 x i8> [[X:%.*]])<br>
> -; CHECK-NEXT: [[M2:%.*]] = call <2 x i8> @llvm.smax.v2i8(<2 x i8> <i8 -127, i8 -127>, <2 x i8> [[M]])<br>
> -; CHECK-NEXT: ret <2 x i8> [[M2]]<br>
> +; CHECK-NEXT: ret <2 x i8> [[M]]<br>
> ;<br>
> %m = call <2 x i8> @llvm.smax.v2i8(<2 x i8> <i8 7, i8 7>, <2 x i8> %x)<br>
> %m2 = call <2 x i8> @llvm.smax.v2i8(<2 x i8> <i8 -127, i8 -127>, <2 x i8> %m)<br>
> @@ -737,8 +727,7 @@ define <2 x i8> @smax_smax_constants_commute1(<2 x i8> %x) {<br>
> define i8 @smax_smax_constants_commute2(i8 %x) {<br>
> ; CHECK-LABEL: @smax_smax_constants_commute2(<br>
> ; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.smax.i8(i8 [[X:%.*]], i8 0)<br>
> -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.smax.i8(i8 [[M]], i8 -1)<br>
> -; CHECK-NEXT: ret i8 [[M2]]<br>
> +; CHECK-NEXT: ret i8 [[M]]<br>
> ;<br>
> %m = call i8 @llvm.smax.i8(i8 %x, i8 0)<br>
> %m2 = call i8 @llvm.smax.i8(i8 %m, i8 -1)<br>
> @@ -748,8 +737,7 @@ define i8 @smax_smax_constants_commute2(i8 %x) {<br>
> define i8 @smax_smax_constants_commute3(i8 %x) {<br>
> ; CHECK-LABEL: @smax_smax_constants_commute3(<br>
> ; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.smax.i8(i8 -1, i8 [[X:%.*]])<br>
> -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.smax.i8(i8 [[M]], i8 -127)<br>
> -; CHECK-NEXT: ret i8 [[M2]]<br>
> +; CHECK-NEXT: ret i8 [[M]]<br>
> ;<br>
> %m = call i8 @llvm.smax.i8(i8 -1, i8 %x)<br>
> %m2 = call i8 @llvm.smax.i8(i8 %m, i8 -127)<br>
> @@ -759,8 +747,7 @@ define i8 @smax_smax_constants_commute3(i8 %x) {<br>
> define <2 x i8> @smin_smin_constants(<2 x i8> %x) {<br>
> ; CHECK-LABEL: @smin_smin_constants(<br>
> ; CHECK-NEXT: [[M:%.*]] = call <2 x i8> @llvm.smin.v2i8(<2 x i8> [[X:%.*]], <2 x i8> <i8 7, i8 7>)<br>
> -; CHECK-NEXT: [[M2:%.*]] = call <2 x i8> @llvm.smin.v2i8(<2 x i8> <i8 undef, i8 9>, <2 x i8> [[M]])<br>
> -; CHECK-NEXT: ret <2 x i8> [[M2]]<br>
> +; CHECK-NEXT: ret <2 x i8> [[M]]<br>
> ;<br>
> %m = call <2 x i8> @llvm.smin.v2i8(<2 x i8> %x, <2 x i8> <i8 7, i8 7>)<br>
> %m2 = call <2 x i8> @llvm.smin.v2i8(<2 x i8> <i8 undef, i8 9>, <2 x i8> %m)<br>
> @@ -770,8 +757,7 @@ define <2 x i8> @smin_smin_constants(<2 x i8> %x) {<br>
> define i8 @smin_smin_constants_commute1(i8 %x) {<br>
> ; CHECK-LABEL: @smin_smin_constants_commute1(<br>
> ; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.smin.i8(i8 -127, i8 [[X:%.*]])<br>
> -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.smin.i8(i8 7, i8 [[M]])<br>
> -; CHECK-NEXT: ret i8 [[M2]]<br>
> +; CHECK-NEXT: ret i8 [[M]]<br>
> ;<br>
> %m = call i8 @llvm.smin.i8(i8 -127, i8 %x)<br>
> %m2 = call i8 @llvm.smin.i8(i8 7, i8 %m)<br>
> @@ -781,8 +767,7 @@ define i8 @smin_smin_constants_commute1(i8 %x) {<br>
> define i8 @smin_smin_constants_commute2(i8 %x) {<br>
> ; CHECK-LABEL: @smin_smin_constants_commute2(<br>
> ; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.smin.i8(i8 [[X:%.*]], i8 -1)<br>
> -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.smin.i8(i8 [[M]], i8 0)<br>
> -; CHECK-NEXT: ret i8 [[M2]]<br>
> +; CHECK-NEXT: ret i8 [[M]]<br>
> ;<br>
> %m = call i8 @llvm.smin.i8(i8 %x, i8 -1)<br>
> %m2 = call i8 @llvm.smin.i8(i8 %m, i8 0)<br>
> @@ -792,14 +777,15 @@ define i8 @smin_smin_constants_commute2(i8 %x) {<br>
> define i8 @smin_smin_constants_commute3(i8 %x) {<br>
> ; CHECK-LABEL: @smin_smin_constants_commute3(<br>
> ; CHECK-NEXT: [[M:%.*]] = call i8 @llvm.smin.i8(i8 -127, i8 [[X:%.*]])<br>
> -; CHECK-NEXT: [[M2:%.*]] = call i8 @llvm.smin.i8(i8 [[M]], i8 -1)<br>
> -; CHECK-NEXT: ret i8 [[M2]]<br>
> +; CHECK-NEXT: ret i8 [[M]]<br>
> ;<br>
> %m = call i8 @llvm.smin.i8(i8 -127, i8 %x)<br>
> %m2 = call i8 @llvm.smin.i8(i8 %m, i8 -1)<br>
> ret i8 %m2<br>
> }<br>
> <br>
> +; Negative test - undef in inner constant must not propagate.<br>
> +<br>
> define <2 x i8> @umin_umin_constants_partial_undef(<2 x i8> %x) {<br>
> ; CHECK-LABEL: @umin_umin_constants_partial_undef(<br>
> ; CHECK-NEXT: [[M:%.*]] = call <2 x i8> @llvm.umin.v2i8(<2 x i8> [[X:%.*]], <2 x i8> <i8 7, i8 undef>)<br>
> @@ -811,6 +797,8 @@ define <2 x i8> @umin_umin_constants_partial_undef(<2 x i8> %x) {<br>
> ret <2 x i8> %m2<br>
> }<br>
> <br>
> +; Negative test - undef in inner constant must not propagate.<br>
> +<br>
> define <2 x i8> @smax_smax_constants_partial_undef(<2 x i8> %x) {<br>
> ; CHECK-LABEL: @smax_smax_constants_partial_undef(<br>
> ; CHECK-NEXT: [[M:%.*]] = call <2 x i8> @llvm.smax.v2i8(<2 x i8> [[X:%.*]], <2 x i8> <i8 undef, i8 10>)<br>
><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>
_______________________________________________<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></div>
</blockquote></div>