<div dir="ltr"><div>Thanks! The IR should be enough to figure this out. I have it down to this so far:</div><div><a href="https://alive2.llvm.org/ce/z/7SdYB4">https://alive2.llvm.org/ce/z/7SdYB4</a></div><div><br></div><div>The transform in this patch seems correct, but something is changing the subsequent shuffle masks, and that's not safe.<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Aug 25, 2020 at 8:13 AM Benjamin Kramer <<a href="mailto:benny.kra@gmail.com">benny.kra@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">The IR was generated by XLA from a JAX unit test. There is no .c input.<br>
<br>
On Tue, Aug 25, 2020 at 1:21 PM Juneyoung Lee<br>
<<a href="mailto:juneyoung.lee@sf.snu.ac.kr" target="_blank">juneyoung.lee@sf.snu.ac.kr</a>> wrote:<br>
><br>
> Hi,<br>
> It seems like something is happening - could you have the source .c file?<br>
><br>
> Thanks,<br>
> Juneyoung<br>
><br>
> On Tue, Aug 25, 2020 at 6:31 PM Benjamin Kramer via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> I'm seeing some dubious miscompiles with this change, looks like<br>
>> something is wrong with poison, going to revert this change.<br>
>><br>
>> Attached test case IR before and after this change, I hope that's<br>
>> enough to see what's going on. Let me know if you need an executable<br>
>> version.<br>
>><br>
>> On Mon, Aug 24, 2020 at 11:00 PM Sanjay Patel via llvm-commits<br>
>> <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
>> ><br>
>> ><br>
>> > Author: Sanjay Patel<br>
>> > Date: 2020-08-24T17:00:16-04:00<br>
>> > New Revision: 557b890ff4f4dd5fa979c232df5b31cf3fef04c1<br>
>> ><br>
>> > URL: <a href="https://github.com/llvm/llvm-project/commit/557b890ff4f4dd5fa979c232df5b31cf3fef04c1" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/557b890ff4f4dd5fa979c232df5b31cf3fef04c1</a><br>
>> > DIFF: <a href="https://github.com/llvm/llvm-project/commit/557b890ff4f4dd5fa979c232df5b31cf3fef04c1.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/557b890ff4f4dd5fa979c232df5b31cf3fef04c1.diff</a><br>
>> ><br>
>> > LOG: [InstCombine] improve demanded element analysis for vector insert-of-extract<br>
>> ><br>
>> > InstCombine currently has odd rules for folding insert-extract chains to shuffles,<br>
>> > so we miss collapsing seemingly simple cases as shown in the tests here.<br>
>> ><br>
>> > But poison makes this not quite as easy as we might have guessed. Alive2 tests to<br>
>> > show the subtle difference (similar to the regression tests):<br>
>> > <a href="https://alive2.llvm.org/ce/z/hp4hv3" rel="noreferrer" target="_blank">https://alive2.llvm.org/ce/z/hp4hv3</a> (this is ok)<br>
>> > <a href="https://alive2.llvm.org/ce/z/ehEWaN" rel="noreferrer" target="_blank">https://alive2.llvm.org/ce/z/ehEWaN</a> (poison leakage)<br>
>> ><br>
>> > SLP tends to create these patterns (as shown in the SLP tests), and this could<br>
>> > help with solving PR16739.<br>
>> ><br>
>> > Differential Revision: <a href="https://reviews.llvm.org/D86460" rel="noreferrer" target="_blank">https://reviews.llvm.org/D86460</a><br>
>> ><br>
>> > Added:<br>
>> ><br>
>> ><br>
>> > Modified:<br>
>> > llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp<br>
>> > llvm/test/Transforms/InstCombine/vec_demanded_elts.ll<br>
>> > llvm/test/Transforms/SLPVectorizer/AArch64/getelementptr.ll<br>
>> > llvm/test/Transforms/SLPVectorizer/X86/operandorder.ll<br>
>> ><br>
>> > Removed:<br>
>> ><br>
>> ><br>
>> ><br>
>> > ################################################################################<br>
>> > diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp<br>
>> > index 382db79cba60..ba9631a83a6f 100644<br>
>> > --- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp<br>
>> > +++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp<br>
>> > @@ -1161,6 +1161,17 @@ Value *InstCombinerImpl::SimplifyDemandedVectorElts(Value *V,<br>
>> > return I->getOperand(0);<br>
>> > }<br>
>> ><br>
>> > + // If we only demand the element that is being inserted and that element<br>
>> > + // was extracted from the same index in another vector with the same type,<br>
>> > + // replace this insert with that other vector.<br>
>> > + Value *Vec;<br>
>> > + if (PreInsertDemandedElts == 0 &&<br>
>> > + match(I->getOperand(1),<br>
>> > + m_ExtractElt(m_Value(Vec), m_SpecificInt(IdxNo))) &&<br>
>> > + Vec->getType() == I->getType()) {<br>
>> > + return Vec;<br>
>> > + }<br>
>> > +<br>
>> > // The inserted element is defined.<br>
>> > UndefElts.clearBit(IdxNo);<br>
>> > break;<br>
>> ><br>
>> > diff --git a/llvm/test/Transforms/InstCombine/vec_demanded_elts.ll b/llvm/test/Transforms/InstCombine/vec_demanded_elts.ll<br>
>> > index bcf5079083bc..b0edfd318a8a 100644<br>
>> > --- a/llvm/test/Transforms/InstCombine/vec_demanded_elts.ll<br>
>> > +++ b/llvm/test/Transforms/InstCombine/vec_demanded_elts.ll<br>
>> > @@ -749,9 +749,7 @@ define <4 x i8> @select_cond_(<4 x i8> %x, <4 x i8> %min, <4 x i1> %cmp, i1 %poi<br>
>> ><br>
>> > define <4 x float> @ins_of_ext(<4 x float> %x, float %y) {<br>
>> > ; CHECK-LABEL: @ins_of_ext(<br>
>> > -; CHECK-NEXT: [[E0:%.*]] = extractelement <4 x float> [[X:%.*]], i32 0<br>
>> > -; CHECK-NEXT: [[I0:%.*]] = insertelement <4 x float> undef, float [[E0]], i32 0<br>
>> > -; CHECK-NEXT: [[I1:%.*]] = insertelement <4 x float> [[I0]], float [[Y:%.*]], i32 1<br>
>> > +; CHECK-NEXT: [[I1:%.*]] = insertelement <4 x float> [[X:%.*]], float [[Y:%.*]], i32 1<br>
>> > ; CHECK-NEXT: [[I2:%.*]] = insertelement <4 x float> [[I1]], float [[Y]], i32 2<br>
>> > ; CHECK-NEXT: [[I3:%.*]] = insertelement <4 x float> [[I2]], float [[Y]], i32 3<br>
>> > ; CHECK-NEXT: ret <4 x float> [[I3]]<br>
>> > @@ -766,11 +764,7 @@ define <4 x float> @ins_of_ext(<4 x float> %x, float %y) {<br>
>> ><br>
>> > define <4 x float> @ins_of_ext_twice(<4 x float> %x, float %y) {<br>
>> > ; CHECK-LABEL: @ins_of_ext_twice(<br>
>> > -; CHECK-NEXT: [[E0:%.*]] = extractelement <4 x float> [[X:%.*]], i32 0<br>
>> > -; CHECK-NEXT: [[I0:%.*]] = insertelement <4 x float> undef, float [[E0]], i32 0<br>
>> > -; CHECK-NEXT: [[E1:%.*]] = extractelement <4 x float> [[X]], i32 1<br>
>> > -; CHECK-NEXT: [[I1:%.*]] = insertelement <4 x float> [[I0]], float [[E1]], i32 1<br>
>> > -; CHECK-NEXT: [[I2:%.*]] = insertelement <4 x float> [[I1]], float [[Y:%.*]], i32 2<br>
>> > +; CHECK-NEXT: [[I2:%.*]] = insertelement <4 x float> [[X:%.*]], float [[Y:%.*]], i32 2<br>
>> > ; CHECK-NEXT: [[I3:%.*]] = insertelement <4 x float> [[I2]], float [[Y]], i32 3<br>
>> > ; CHECK-NEXT: ret <4 x float> [[I3]]<br>
>> > ;<br>
>> > @@ -783,6 +777,9 @@ define <4 x float> @ins_of_ext_twice(<4 x float> %x, float %y) {<br>
>> > ret <4 x float> %i3<br>
>> > }<br>
>> ><br>
>> > +; Negative test - element 3 of the result must be undef to be poison safe.<br>
>> > +; TODO: Could convert insert/extract to identity shuffle with undef mask elements.<br>
>> > +<br>
>> > define <4 x float> @ins_of_ext_wrong_demand(<4 x float> %x, float %y) {<br>
>> > ; CHECK-LABEL: @ins_of_ext_wrong_demand(<br>
>> > ; CHECK-NEXT: [[E0:%.*]] = extractelement <4 x float> [[X:%.*]], i32 0<br>
>> > @@ -798,6 +795,9 @@ define <4 x float> @ins_of_ext_wrong_demand(<4 x float> %x, float %y) {<br>
>> > ret <4 x float> %i2<br>
>> > }<br>
>> ><br>
>> > +; Negative test - can't replace i0 with x.<br>
>> > +; TODO: Could convert insert/extract to identity shuffle with undef mask elements.<br>
>> > +<br>
>> > define <4 x float> @ins_of_ext_wrong_type(<5 x float> %x, float %y) {<br>
>> > ; CHECK-LABEL: @ins_of_ext_wrong_type(<br>
>> > ; CHECK-NEXT: [[E0:%.*]] = extractelement <5 x float> [[X:%.*]], i32 0<br>
>> ><br>
>> > diff --git a/llvm/test/Transforms/SLPVectorizer/AArch64/getelementptr.ll b/llvm/test/Transforms/SLPVectorizer/AArch64/getelementptr.ll<br>
>> > index 67b6e04ee391..3dae1a7ca6d3 100644<br>
>> > --- a/llvm/test/Transforms/SLPVectorizer/AArch64/getelementptr.ll<br>
>> > +++ b/llvm/test/Transforms/SLPVectorizer/AArch64/getelementptr.ll<br>
>> > @@ -51,13 +51,13 @@ define i32 @getelementptr_4x32(i32* nocapture readonly %g, i32 %n, i32 %x, i32 %<br>
>> > ; CHECK-NEXT: [[TMP2:%.*]] = insertelement <4 x i32> [[TMP1]], i32 [[Z:%.*]], i32 3<br>
>> > ; CHECK-NEXT: br label [[FOR_BODY:%.*]]<br>
>> > ; CHECK: for.cond.cleanup.loopexit:<br>
>> > -; CHECK-NEXT: [[TMP3:%.*]] = extractelement <2 x i32> [[TMP21:%.*]], i32 1<br>
>> > +; CHECK-NEXT: [[TMP3:%.*]] = extractelement <2 x i32> [[TMP20:%.*]], i32 1<br>
>> > ; CHECK-NEXT: br label [[FOR_COND_CLEANUP]]<br>
>> > ; CHECK: for.cond.cleanup:<br>
>> > ; CHECK-NEXT: [[SUM_0_LCSSA:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[TMP3]], [[FOR_COND_CLEANUP_LOOPEXIT:%.*]] ]<br>
>> > ; CHECK-NEXT: ret i32 [[SUM_0_LCSSA]]<br>
>> > ; CHECK: for.body:<br>
>> > -; CHECK-NEXT: [[TMP4:%.*]] = phi <2 x i32> [ zeroinitializer, [[FOR_BODY_PREHEADER]] ], [ [[TMP21]], [[FOR_BODY]] ]<br>
>> > +; CHECK-NEXT: [[TMP4:%.*]] = phi <2 x i32> [ zeroinitializer, [[FOR_BODY_PREHEADER]] ], [ [[TMP20]], [[FOR_BODY]] ]<br>
>> > ; CHECK-NEXT: [[TMP5:%.*]] = extractelement <2 x i32> [[TMP4]], i32 0<br>
>> > ; CHECK-NEXT: [[T4:%.*]] = shl nsw i32 [[TMP5]], 1<br>
>> > ; CHECK-NEXT: [[TMP6:%.*]] = insertelement <4 x i32> undef, i32 [[T4]], i32 0<br>
>> > @@ -83,12 +83,11 @@ define i32 @getelementptr_4x32(i32* nocapture readonly %g, i32 %n, i32 %x, i32 %<br>
>> > ; CHECK-NEXT: [[TMP17:%.*]] = sext i32 [[TMP16]] to i64<br>
>> > ; CHECK-NEXT: [[ARRAYIDX15:%.*]] = getelementptr inbounds i32, i32* [[G]], i64 [[TMP17]]<br>
>> > ; CHECK-NEXT: [[T12:%.*]] = load i32, i32* [[ARRAYIDX15]], align 4<br>
>> > -; CHECK-NEXT: [[TMP18:%.*]] = insertelement <2 x i32> undef, i32 [[TMP5]], i32 0<br>
>> > -; CHECK-NEXT: [[TMP19:%.*]] = insertelement <2 x i32> [[TMP18]], i32 [[ADD11]], i32 1<br>
>> > -; CHECK-NEXT: [[TMP20:%.*]] = insertelement <2 x i32> <i32 1, i32 undef>, i32 [[T12]], i32 1<br>
>> > -; CHECK-NEXT: [[TMP21]] = add nsw <2 x i32> [[TMP19]], [[TMP20]]<br>
>> > -; CHECK-NEXT: [[TMP22:%.*]] = extractelement <2 x i32> [[TMP21]], i32 0<br>
>> > -; CHECK-NEXT: [[EXITCOND:%.*]] = icmp eq i32 [[TMP22]], [[N]]<br>
>> > +; CHECK-NEXT: [[TMP18:%.*]] = insertelement <2 x i32> [[TMP4]], i32 [[ADD11]], i32 1<br>
>> > +; CHECK-NEXT: [[TMP19:%.*]] = insertelement <2 x i32> <i32 1, i32 undef>, i32 [[T12]], i32 1<br>
>> > +; CHECK-NEXT: [[TMP20]] = add nsw <2 x i32> [[TMP18]], [[TMP19]]<br>
>> > +; CHECK-NEXT: [[TMP21:%.*]] = extractelement <2 x i32> [[TMP20]], i32 0<br>
>> > +; CHECK-NEXT: [[EXITCOND:%.*]] = icmp eq i32 [[TMP21]], [[N]]<br>
>> > ; CHECK-NEXT: br i1 [[EXITCOND]], label [[FOR_COND_CLEANUP_LOOPEXIT]], label [[FOR_BODY]]<br>
>> > ;<br>
>> > entry:<br>
>> > @@ -157,13 +156,13 @@ define i32 @getelementptr_2x32(i32* nocapture readonly %g, i32 %n, i32 %x, i32 %<br>
>> > ; CHECK-NEXT: [[TMP1:%.*]] = insertelement <2 x i32> [[TMP0]], i32 [[Z:%.*]], i32 1<br>
>> > ; CHECK-NEXT: br label [[FOR_BODY:%.*]]<br>
>> > ; CHECK: for.cond.cleanup.loopexit:<br>
>> > -; CHECK-NEXT: [[TMP2:%.*]] = extractelement <2 x i32> [[TMP18:%.*]], i32 1<br>
>> > +; CHECK-NEXT: [[TMP2:%.*]] = extractelement <2 x i32> [[TMP17:%.*]], i32 1<br>
>> > ; CHECK-NEXT: br label [[FOR_COND_CLEANUP]]<br>
>> > ; CHECK: for.cond.cleanup:<br>
>> > ; CHECK-NEXT: [[SUM_0_LCSSA:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[TMP2]], [[FOR_COND_CLEANUP_LOOPEXIT:%.*]] ]<br>
>> > ; CHECK-NEXT: ret i32 [[SUM_0_LCSSA]]<br>
>> > ; CHECK: for.body:<br>
>> > -; CHECK-NEXT: [[TMP3:%.*]] = phi <2 x i32> [ zeroinitializer, [[FOR_BODY_PREHEADER]] ], [ [[TMP18]], [[FOR_BODY]] ]<br>
>> > +; CHECK-NEXT: [[TMP3:%.*]] = phi <2 x i32> [ zeroinitializer, [[FOR_BODY_PREHEADER]] ], [ [[TMP17]], [[FOR_BODY]] ]<br>
>> > ; CHECK-NEXT: [[TMP4:%.*]] = extractelement <2 x i32> [[TMP3]], i32 0<br>
>> > ; CHECK-NEXT: [[T4:%.*]] = shl nsw i32 [[TMP4]], 1<br>
>> > ; CHECK-NEXT: [[TMP5:%.*]] = sext i32 [[T4]] to i64<br>
>> > @@ -188,12 +187,11 @@ define i32 @getelementptr_2x32(i32* nocapture readonly %g, i32 %n, i32 %x, i32 %<br>
>> > ; CHECK-NEXT: [[TMP14:%.*]] = sext i32 [[TMP13]] to i64<br>
>> > ; CHECK-NEXT: [[ARRAYIDX15:%.*]] = getelementptr inbounds i32, i32* [[G]], i64 [[TMP14]]<br>
>> > ; CHECK-NEXT: [[T12:%.*]] = load i32, i32* [[ARRAYIDX15]], align 4<br>
>> > -; CHECK-NEXT: [[TMP15:%.*]] = insertelement <2 x i32> undef, i32 [[TMP4]], i32 0<br>
>> > -; CHECK-NEXT: [[TMP16:%.*]] = insertelement <2 x i32> [[TMP15]], i32 [[ADD11]], i32 1<br>
>> > -; CHECK-NEXT: [[TMP17:%.*]] = insertelement <2 x i32> <i32 1, i32 undef>, i32 [[T12]], i32 1<br>
>> > -; CHECK-NEXT: [[TMP18]] = add nsw <2 x i32> [[TMP16]], [[TMP17]]<br>
>> > -; CHECK-NEXT: [[TMP19:%.*]] = extractelement <2 x i32> [[TMP18]], i32 0<br>
>> > -; CHECK-NEXT: [[EXITCOND:%.*]] = icmp eq i32 [[TMP19]], [[N]]<br>
>> > +; CHECK-NEXT: [[TMP15:%.*]] = insertelement <2 x i32> [[TMP3]], i32 [[ADD11]], i32 1<br>
>> > +; CHECK-NEXT: [[TMP16:%.*]] = insertelement <2 x i32> <i32 1, i32 undef>, i32 [[T12]], i32 1<br>
>> > +; CHECK-NEXT: [[TMP17]] = add nsw <2 x i32> [[TMP15]], [[TMP16]]<br>
>> > +; CHECK-NEXT: [[TMP18:%.*]] = extractelement <2 x i32> [[TMP17]], i32 0<br>
>> > +; CHECK-NEXT: [[EXITCOND:%.*]] = icmp eq i32 [[TMP18]], [[N]]<br>
>> > ; CHECK-NEXT: br i1 [[EXITCOND]], label [[FOR_COND_CLEANUP_LOOPEXIT]], label [[FOR_BODY]]<br>
>> > ;<br>
>> > entry:<br>
>> ><br>
>> > diff --git a/llvm/test/Transforms/SLPVectorizer/X86/operandorder.ll b/llvm/test/Transforms/SLPVectorizer/X86/operandorder.ll<br>
>> > index 65f26abc87b4..70f5d105e654 100644<br>
>> > --- a/llvm/test/Transforms/SLPVectorizer/X86/operandorder.ll<br>
>> > +++ b/llvm/test/Transforms/SLPVectorizer/X86/operandorder.ll<br>
>> > @@ -183,13 +183,11 @@ define void @vecload_vs_broadcast5(double * noalias %from, double * noalias %to,<br>
>> > ; CHECK-NEXT: [[P:%.*]] = phi double [ 1.000000e+00, [[LP]] ], [ 0.000000e+00, [[ENTRY:%.*]] ]<br>
>> > ; CHECK-NEXT: [[TMP0:%.*]] = bitcast double* [[FROM:%.*]] to <2 x double>*<br>
>> > ; CHECK-NEXT: [[TMP1:%.*]] = load <2 x double>, <2 x double>* [[TMP0]], align 4<br>
>> > -; CHECK-NEXT: [[TMP2:%.*]] = extractelement <2 x double> [[TMP1]], i32 0<br>
>> > -; CHECK-NEXT: [[TMP3:%.*]] = insertelement <2 x double> undef, double [[TMP2]], i32 0<br>
>> > -; CHECK-NEXT: [[TMP4:%.*]] = insertelement <2 x double> [[TMP3]], double [[P]], i32 1<br>
>> > -; CHECK-NEXT: [[TMP5:%.*]] = shufflevector <2 x double> [[TMP1]], <2 x double> undef, <2 x i32> <i32 1, i32 0><br>
>> > -; CHECK-NEXT: [[TMP6:%.*]] = fadd <2 x double> [[TMP4]], [[TMP5]]<br>
>> > -; CHECK-NEXT: [[TMP7:%.*]] = bitcast double* [[TO:%.*]] to <2 x double>*<br>
>> > -; CHECK-NEXT: store <2 x double> [[TMP6]], <2 x double>* [[TMP7]], align 4<br>
>> > +; CHECK-NEXT: [[TMP2:%.*]] = insertelement <2 x double> [[TMP1]], double [[P]], i32 1<br>
>> > +; CHECK-NEXT: [[TMP3:%.*]] = shufflevector <2 x double> [[TMP1]], <2 x double> undef, <2 x i32> <i32 1, i32 0><br>
>> > +; CHECK-NEXT: [[TMP4:%.*]] = fadd <2 x double> [[TMP2]], [[TMP3]]<br>
>> > +; CHECK-NEXT: [[TMP5:%.*]] = bitcast double* [[TO:%.*]] to <2 x double>*<br>
>> > +; CHECK-NEXT: store <2 x double> [[TMP4]], <2 x double>* [[TMP5]], align 4<br>
>> > ; CHECK-NEXT: br i1 undef, label [[LP]], label [[EXT:%.*]]<br>
>> > ; CHECK: ext:<br>
>> > ; CHECK-NEXT: ret void<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>
><br>
><br>
><br>
> --<br>
><br>
> Juneyoung Lee<br>
> Software Foundation Lab, Seoul National University<br>
</blockquote></div>