<div>Hi Michael,<br><br>+llvmdev,Hal,Nadav<br><br>For testing, I was currently thinking of a two pronged approach. Lit tests as you suggest with a dummy pass, probably with command line options to define what transform to do, and unit tests to test the delegate behaviour and return values. <br><br>I'll try and produce a mega patch with at least the loop vectoriser moved over, then split it up again after review.<br><br>Cheers,<br><br>James</div><br><div class="gmail_quote"><div dir="ltr">On Tue, 28 Jul 2015 at 20:27, Michael Zolotukhin <<a href="mailto:mzolotukhin@apple.com">mzolotukhin@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>BTW, I think this thread should be on LLVM-dev, and I’d also CC Arnold, Hal, and Nadav - their feedback would be very valuable here.</div></div><div style="word-wrap:break-word"><div><br></div><div>Michael</div></div><div style="word-wrap:break-word"><br><div><blockquote type="cite"><div>On Jul 28, 2015, at 12:21 PM, Mikhail Zolotukhin <<a href="mailto:mzolotukhin@apple.com" target="_blank">mzolotukhin@apple.com</a>> wrote:</div><br><div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">Hi James,</div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br></div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">Thanks for your comments!</div><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><blockquote type="cite"><div>On Jul 28, 2015, at 1:08 AM, James Molloy <<a href="mailto:james@jamesmolloy.co.uk" target="_blank">james@jamesmolloy.co.uk</a>> wrote:</div><br><div><div dir="ltr">Hi Michael,<div><br></div><div>Thanks for the feedback.</div><div><br></div><div>> <span style="font-size:13.1999998092651px;line-height:19.7999992370605px">As a show-case, how does it help to simplify the aforementioned createEmptyBlock?</span></div><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px"><br></span></div><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px">That's a good example. If you look at </span><a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11530&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=VnVEJNalm7wL1msEOqLtriWigVs6sRVqC3Z1wsH2tw4&s=zH8LxgRmsAyXqnBEYDvEp-VfGrZKo6oCWhUw9CEHDCI&e=" target="_blank">http://reviews.llvm.org/D11530</a><span> </span>line 916 in the .cpp file, there's a function "versionWidenAndInterleaveLoop" that does basically createEmptyBlock(). It also does more stuff - it performs the loop interleave too - but as an example showcase it should do.</div><div><br></div><div>For the vectorizer, I think the cleanest thing would be to add a delegate hook that's called whenever an instruction is about to be cloned. Then, the vectorizer could override that hook and provide its own implementation (instead of cloning an instruction completely, it'd replace it with a vector variant. So the entire main body of the vectorizer would look something like this:</div><div><br></div><div><span style="line-height:1.5;font-size:13.1999998092651px">struct Delegate : public LoopEditor::Delegate {</span><br></div><div>  Value *hookCloneInstruction(Instruction *OldI, IRBuilder<> &IRB) {</div><div>    if (isControlFlowInst(OldI)) return IRB.CreateExtractElement; // Don't vectorize branches</div><div><span style="line-height:1.5;font-size:13.1999998092651px">    return createVectorVersionOfInstruction(OldI, VF, IRB); // Defined somewhere in LoopVectorize</span><br></div><div><span style="line-height:1.5;font-size:13.1999998092651px">  }</span></div><div><span style="line-height:1.5;font-size:13.1999998092651px">};</span></div><div><span style="line-height:1.5;font-size:13.1999998092651px">BasicBlock *PredBB;</span></div><div><span style="line-height:1.5;font-size:13.1999998092651px">Delegate D;</span></div><div><div style="font-size:13.1999998092651px;line-height:19.7999992370605px">LoopEditor LE(L, DT, SE, LI);</div></div><div><span style="line-height:1.5;font-size:13.1999998092651px">LoopEditor VectorLE = LE.versionWidenAndInterleaveLoop(UF, PredBB, &D);</span></div><div><span style="line-height:1.5;font-size:13.1999998092651px">VectorLE.widen(VF); // Widen further, so it's widened by VF*UF. Only affects induction variable steps and trip count.</span></div><div><span style="line-height:1.5;font-size:13.1999998092651px"><br></span></div><div><span style="line-height:1.5;font-size:13.1999998092651px">How does that look to you?</span></div></div></div></blockquote>That looks good, I always wanted to factor this stuff out too:)<br><blockquote type="cite"><div><div dir="ltr"><div><span style="line-height:1.5;font-size:13.1999998092651px"><br></span></div><div><span style="line-height:1.5;font-size:13.1999998092651px">> </span><span style="font-size:13.1999998092651px;line-height:19.7999992370605px">Also, I think it’s very important to explicitly define requirements for loops we can handle, and what we do with other loops. E.g. what do we do if a loop has multiple back-edges? What do we do if a loop body has complicated control-flow?</span></div><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px"><br></span></div><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px">Good point, agreed. I think we need to handle loops in LoopSimplify form and LCSSA form at least. Some functions may not be able to handle arbitrary control flow but most should be agnostic to it (for example making sure control flow is correct in a vectorized loop, should be the user's job not LoopEditor's). But defining exactly what loop type each function supports and ensuring it does support that is a good idea.</span></div><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px"><br></span></div><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px">I'm also trying to work out what's the best way of getting this in-tree. It's a lot of code and I don't want to dump it in one go, but getting pre-commit review on every little step would make it take forever. Do you have any suggestions as to methodology? I was thinking getting review on the header (API), then committing that with stub implementations and implementing them bit by bit from there...</span></div></div></div></blockquote>I agree, and this way seems reasonable to me. However, it's really tempting to see what we end up with before we commit to it - so, if you have some final patch with this API applied to most of its users (even if this patch is huge), it would be cool if you share it.</div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br></div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">Also, how will we test it? Should we create a dummy-pass that will invoke these transforms on a given loop (without any cost models, just doing what you ask it to do)?</div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br></div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">Michael<br><blockquote type="cite"><div><div dir="ltr"><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px"><br></span></div><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px">Cheers,</span></div><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px"><br></span></div><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px">James</span></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, 27 Jul 2015 at 20:23 Michael Zolotukhin <<a href="mailto:mzolotukhin@apple.com" target="_blank">mzolotukhin@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word">Hi James,<div><br></div><div><div><blockquote type="cite"></blockquote></div></div></div><div style="word-wrap:break-word"><div><div><blockquote type="cite"><div>On Jul 27, 2015, at 10:04 AM, James Molloy <<a href="mailto:james@jamesmolloy.co.uk" target="_blank">james@jamesmolloy.co.uk</a>> wrote:</div><br></blockquote></div></div></div><div style="word-wrap:break-word"><div><div><blockquote type="cite"><div><div dir="ltr">Hi all,<div><br></div><div>LLVM currently lacks a single coherent way to edit, modify and transform loops. We have the Loop abstraction that provides some functionality, and we have a cluster of functions and abstractions in LoopUtils, but each is fairly cumbersome to use. There's also VersionedLoop, but that's quite specific and not really extendable/composable.</div><div><br></div><div>I've recently been trying to do some prototyping of a high level loop optimization (unroll-and-jam) and ended up with quite a bit of spaghetti code doing what should be simple transforms (as another example, look at LoopVectorize::createEmptyBlock()!)</div></div></div></blockquote></div></div></div><div style="word-wrap:break-word"><div><div><blockquote type="cite"></blockquote>Totally agree here!<br><blockquote type="cite"><div><div dir="ltr"></div></div></blockquote></div></div></div><div style="word-wrap:break-word"><div><div><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>So I decided to prototype an API for performing more high level operations on LLVM Loops - cloning, widening, interleaving, stitching the output of one loop into another loop, setting loop trip counts, peeling... and all while keeping loopinfo, domtree and SCEV updated.</div><div><br></div></div></div></blockquote></div></div></div><div style="word-wrap:break-word"><div><div><blockquote type="cite"><div><div dir="ltr"><div>I've uploaded my current prototype to Phab here: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11530&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=3t7gCOgkqnA-65i6cez_eslTon3hxx5XJKhMBw98kv0&s=yCaxJfCogeqwUH-wlsdHFPHktniLwG3-cZnq1yciR04&e=" target="_blank">http://reviews.llvm.org/D11530</a></div></div></div></blockquote></div></div></div><div style="word-wrap:break-word"><div><div><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>It's still at the prototype stage so there are lots of things that need fixing - sorting out the testing story being top of my list. But I'm looking for feedback on:</div><div>  * Is this a useful abstraction? Is it something the community would like to see in-tree (eventually)?</div></div></div></blockquote></div></div></div><div style="word-wrap:break-word"><div><div><blockquote type="cite"><div><div dir="ltr"></div></div></blockquote>I think it could be very useful - depending on how easy it would be to use. As a show-case, how does it help to simplify the aforementioned createEmptyBlock?</div><div></div></div></div><div style="word-wrap:break-word"><div><div><br><blockquote type="cite"><div><div dir="ltr"><div>  * Does the API design make sense? Are there obvious things I've missed that people need?</div></div></div></blockquote></div></div></div><div style="word-wrap:break-word"><div><div>From the first glance, it seems reasonable. One probably missing thing: we have peelAfter, but we may also need peelBefore (it would be helpful if we create prologues for alignment for instance).</div><div><br></div><div>Also, I think it’s very important to explicitly define requirements for loops we can handle, and what we do with other loops. E.g. what do we do if a loop has multiple back-edges? What do we do if a loop body has complicated control-flow?</div><div><br></div><div>Thanks,</div><div>Michael</div><div><br><blockquote type="cite"><div></div></blockquote></div></div></div><div style="word-wrap:break-word"><div><div><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>I've thought about the use cases of the loop vectorizer, loop distribution, loop unrolling, loop versioning, and I think the current design can solve their use cases fairly elegantly. I've used this API to write a fairly sophisticated unroll-and-jam transform in very few lines too.</div><div><br></div><div>Any feedback would be gratefully appreciated!</div><div><br></div><div>Cheers,</div><div><br></div><div>James</div></div></div></blockquote></div></div></div><div style="word-wrap:break-word"><div><div><blockquote type="cite"><div>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote></div></div></div></blockquote></div></div></blockquote></div></div></blockquote></div><br></div></blockquote></div>