<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Suggestions for alternatives, then? As it was before, the code was massively pessimizing ARM codegen. Unless we’re willing to add target information here (see FIXME), IMO the code here should if anything be even more aggressive and it’s the target’s responsibility to split things back up.<div><br></div><div>I did check X86 codegen for a variety of test cases and always got better results. Entirely possible, likely even, there’s cases I missed of course.</div><div><br></div><div>-Jim</div><div><div><div><br></div><div><br></div><div>On Apr 30, 2013, at 2:00 PM, Nadav Rotem <<a href="mailto:nrotem@apple.com">nrotem@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Jim, <div><br></div><div>AVX does not allow cross-lane shuffles within a single vector.  Your change can potentially introduce regressions for hand-written code that assumes the current LLVM behavior. </div><div><br></div><div>Thanks,</div><div>NAdav</div><div><br><div><div>On Apr 30, 2013, at 1:43 PM, Jim Grosbach <<a href="mailto:grosbach@apple.com">grosbach@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Author: grosbach<br>Date: Tue Apr 30 15:43:52 2013<br>New Revision: 180802<br><br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=180802&view=rev">http://llvm.org/viewvc/llvm-project?rev=180802&view=rev</a><br>Log:<br>InstCombine: Fold more shuffles of shuffles.<br><br>Always fold a shuffle-of-shuffle into a single shuffle when there's only one<br>input vector in the first place. Continue to be more conservative when there's<br>multiple inputs.<br><br><a href="rdar://13402653">rdar://13402653</a><br>PR15866<br><br>Modified:<br>   llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp<br>   llvm/trunk/test/Transforms/BBVectorize/simple.ll<br>   llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll<br><br>Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp?rev=180802&r1=180801&r2=180802&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp?rev=180802&r1=180801&r2=180802&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp (original)<br>+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp Tue Apr 30 15:43:52 2013<br>@@ -614,11 +614,16 @@ Instruction *InstCombiner::visitShuffleV<br>  // we are absolutely afraid of producing a shuffle mask not in the input<br>  // program, because the code gen may not be smart enough to turn a merged<br>  // shuffle into two specific shuffles: it may produce worse code.  As such,<br>-  // we only merge two shuffles if the result is either a splat or one of the<br>-  // input shuffle masks.  In this case, merging the shuffles just removes<br>-  // one instruction, which we know is safe.  This is good for things like<br>+  // we only merge two shuffles if the result is a splat, one of the input<br>+  // input shuffle masks, or if there's only one input to the shuffle.<br>+  // In this case, merging the shuffles just removes one instruction, which<br>+  // we know is safe.  This is good for things like<br>  // turning: (splat(splat)) -> splat, or<br>  // merge(V[0..n], V[n+1..2n]) -> V[0..2n]<br>+  //<br>+  // FIXME: This is almost certainly far, far too conservative. We should<br>+  // have a better model. Perhaps a TargetTransformInfo hook to ask whether<br>+  // a shuffle is considered OK?<br>  ShuffleVectorInst* LHSShuffle = dyn_cast<ShuffleVectorInst>(LHS);<br>  ShuffleVectorInst* RHSShuffle = dyn_cast<ShuffleVectorInst>(RHS);<br>  if (LHSShuffle)<br>@@ -743,8 +748,10 @@ Instruction *InstCombiner::visitShuffleV<br>  }<br><br>  // If the result mask is equal to one of the original shuffle masks,<br>-  // or is a splat, do the replacement.<br>-  if (isSplat || newMask == LHSMask || newMask == RHSMask || newMask == Mask) {<br>+  // or is a splat, do the replacement. Similarly, if there is only one<br>+  // input vector, go ahead and do the folding.<br>+  if (isSplat || newMask == LHSMask || newMask == RHSMask || newMask == Mask ||<br>+      isa<UndefValue>(RHS)) {<br>    SmallVector<Constant*, 16> Elts;<br>    Type *Int32Ty = Type::getInt32Ty(SVI.getContext());<br>    for (unsigned i = 0, e = newMask.size(); i != e; ++i) {<br><br>Modified: llvm/trunk/test/Transforms/BBVectorize/simple.ll<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/BBVectorize/simple.ll?rev=180802&r1=180801&r2=180802&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/BBVectorize/simple.ll?rev=180802&r1=180801&r2=180802&view=diff</a><br>==============================================================================<br>--- llvm/trunk/test/Transforms/BBVectorize/simple.ll (original)<br>+++ llvm/trunk/test/Transforms/BBVectorize/simple.ll Tue Apr 30 15:43:52 2013<br>@@ -139,11 +139,10 @@ define <8 x i8> @test6(<8 x i8> %A1, <8<br>; CHECK: %Z1 = add <16 x i8> %Y1, %X1.v.i1<br>        %Q1 = shufflevector <8 x i8> %Z1, <8 x i8> %Z2, <8 x i32> <i32 15, i32 8, i32 6, i32 1, i32 13, i32 10, i32 4, i32 3><br>        %Q2 = shufflevector <8 x i8> %Z2, <8 x i8> %Z2, <8 x i32> <i32 6, i32 7, i32 0, i32 1, i32 2, i32 4, i32 4, i32 1><br>-; CHECK: %Q1.v.i1 = shufflevector <16 x i8> %Z1, <16 x i8> undef, <16 x i32> <i32 8, i32 undef, i32 10, i32 undef, i32 undef, i32 13, i32 undef, i32 15, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef><br>-; CHECK: %Q1 = shufflevector <16 x i8> %Z1, <16 x i8> %Q1.v.i1, <16 x i32> <i32 23, i32 16, i32 6, i32 1, i32 21, i32 18, i32 4, i32 3, i32 14, i32 15, i32 8, i32 9, i32 10, i32 12, i32 12, i32 9><br>-<span class="Apple-tab-span" style="white-space: pre;">     </span>%R  = mul <8 x i8> %Q1, %Q2<br>-; CHECK: %Q1.v.r1 = shufflevector <16 x i8> %Q1, <16 x i8> undef, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7><br>-; CHECK: %Q1.v.r2 = shufflevector <16 x i8> %Q1, <16 x i8> undef, <8 x i32> <i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15><br>+        %R  = mul <8 x i8> %Q1, %Q2<br>+; CHECK:  %Q1.v.i1 = shufflevector <16 x i8> %Z1, <16 x i8> undef, <16 x i32> <i32 8, i32 undef, i32 10, i32 undef, i32 undef, i32 13, i32 undef, i32 15, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef><br>+; CHECK:  %Q1.v.r1 = shufflevector <16 x i8> %Z1, <16 x i8> %Q1.v.i1, <8 x i32> <i32 23, i32 16, i32 6, i32 1, i32 21, i32 18, i32 4, i32 3><br>+; CHECK:  %Q1.v.r2 = shufflevector <16 x i8> %Z1, <16 x i8> undef, <8 x i32> <i32 14, i32 15, i32 8, i32 9, i32 10, i32 12, i32 12, i32 9><br>; CHECK: %R = mul <8 x i8> %Q1.v.r1, %Q1.v.r2<br><span class="Apple-tab-span" style="white-space: pre;">      </span>ret <8 x i8> %R<br>; CHECK: ret <8 x i8> %R<br><br>Modified: llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll?rev=180802&r1=180801&r2=180802&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll?rev=180802&r1=180801&r2=180802&view=diff</a><br>==============================================================================<br>--- llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll (original)<br>+++ llvm/trunk/test/Transforms/InstCombine/vec_shuffle.ll Tue Apr 30 15:43:52 2013<br>@@ -86,14 +86,14 @@ define <4 x i8> @test9(<16 x i8> %tmp6)<br>}<br><br>; Same as test9, but make sure that "undef" mask values are not confused with<br>-; mask values of 2*N, where N is the mask length.  These shuffles should not<br>-; be folded (because [8,9,4,8] may not be a mask supported by the target).<br>-define <4 x i8> @test9a(<16 x i8> %tmp6) nounwind {<br>+; mask values of 2*N, where N is the mask length of the result.  Make sure when<br>+; folding these shuffles that 'undef' mask values stay that way in the result<br>+; instead of getting mapped to the 2*N'th entry of the source.<br>+define <4 x i8> @test9a(<16 x i8> %in, <16 x i8> %in2) nounwind {<br>; CHECK: @test9a<br>-; CHECK-NEXT: shufflevector<br>-; CHECK-NEXT: shufflevector<br>+; CHECK-NEXT: shufflevector <16 x i8> %in, <16 x i8> %in2, <4 x i32> <i32 16, i32 9, i32 4, i32 undef><br>; CHECK-NEXT: ret<br>-<span class="Apple-tab-span" style="white-space: pre;">     </span>%tmp7 = shufflevector <16 x i8> %tmp6, <16 x i8> undef, <4 x i32> < i32 undef, i32 9, i32 4, i32 8 ><span class="Apple-tab-span" style="white-space: pre;">      </span><span class="Apple-tab-span" style="white-space: pre;">  </span>; <<4 x i8>> [#uses=1]<br>+<span class="Apple-tab-span" style="white-space: pre;">     </span>%tmp7 = shufflevector <16 x i8> %in, <16 x i8> %in2, <4 x i32> < i32 undef, i32 9, i32 4, i32 16 ><span class="Apple-tab-span" style="white-space: pre;">        </span><span class="Apple-tab-span" style="white-space: pre;">  </span>; <<4 x i8>> [#uses=1]<br><span class="Apple-tab-span" style="white-space: pre;">      </span>%tmp9 = shufflevector <4 x i8> %tmp7, <4 x i8> undef, <4 x i32> < i32 3, i32 1, i32 2, i32 0 ><span class="Apple-tab-span" style="white-space: pre;">    </span><span class="Apple-tab-span" style="white-space: pre;">  </span>; <<4 x i8>> [#uses=1]<br><span class="Apple-tab-span" style="white-space: pre;">      </span>ret <4 x i8> %tmp9<br>}<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote></div></div></div></blockquote></div><br></div></body></html>