<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Feb 4, 2015, at 6:14 PM, Hal Finkel <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; float: none; display: inline !important;" class="">----- Original Message -----</span><br style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;" class="">From: "Adam Nemet" <<a href="mailto:anemet@apple.com" class="">anemet@apple.com</a>><br class="">To: <a href="mailto:anemet@apple.com" class="">anemet@apple.com</a>, <a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>, <a href="mailto:nrotem@apple.com" class="">nrotem@apple.com</a>, <a href="mailto:aschwaighofer@apple.com" class="">aschwaighofer@apple.com</a>, <a href="mailto:chandlerc@gmail.com" class="">chandlerc@gmail.com</a><br class="">Cc: <a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">Sent: Wednesday, February 4, 2015 5:58:45 PM<br class="">Subject: [PATCH] IRBuilder: Allow retrieval of the inserted instructions<br class=""><br class="">Hi hfinkel, nadav, aschwaighofer, chandlerc,<br class=""><br class="">This is an RFC.<br class=""><br class="">When the Loop Vectorizer builds the instruction sequence for checks<br class="">it tries<br class="">to determine the first instruction that was emitted in the current<br class="">block.<br class="">This is then used to split the block.<br class=""></blockquote><br style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;" class=""><span style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; float: none; display: inline !important;" class="">Silly question: Why don't we split the block first?</span><br style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;" class=""></div></blockquote><div><br class=""></div><div>Because we may end up not needing any checks in which case we don’t split. The decision of whether checks are necessary is local to both addRuntimeCheck and addStrideCheck.</div><div><br class=""></div><div>Adam </div><br class=""><blockquote type="cite" class=""><div class=""><span style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; float: none; display: inline !important;" class="">-Hal</span><br style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;" class=""><br style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;" class=""><br class="">It uses a custom solution for this implemented in the static function<br class="">getFirstInst. The pattern is something like:<br class=""><br class="">Value *V = IRBuilder.CreateBlah(...);<br class="">FirstInst = getFirstInst(FirstInst, V, Loc)<br class="">Value *V2 = IRBuilder.CreateBlah(...);<br class="">FirstInst = getFirstInst(FirstInst, V2, Loc);<br class=""><br class="">(Since CreateBlah may return a constant we may not generate the first<br class="">instruction for V.)<br class=""><br class="">For the LoopAccessAnalysis work I need this to be made global so I<br class="">was<br class="">thinking how to make it more generic. My idea was to change the<br class="">approach and<br class="">rather than repeatedly checking whether we had emitted the first<br class="">instruction,<br class="">remember the predecessor of the insertion point in the IRBuilder.<br class="">Subsequently, when the first emitted instruction is queried I return<br class="">the<br class="">successor of the saved predecessor instruction.<br class=""><br class="">Conceptually:<br class=""><br class="">IRBuilder<>::Marker Mark(IRBuilder.getInsertPoint());<br class=""><br class="">marks the next instruction that will be inserted at the current<br class="">insertion<br class="">point. After inserting instructions into IRBuilder, you can retrieve<br class="">the<br class="">first instruction inserted with Mark.get() (perhaps Mark.first()<br class="">would be a<br class="">better name).<br class=""><br class="">The patch also contains the changes to make use of this in LV and<br class="">then asserts<br class="">that the new result is at least as good as the one from getFirstInst.<br class="">The<br class="">assert does not fail on things I tried so far.<br class=""><br class="">I went back and forth on this but I think it's better to associate<br class="">the marker<br class="">with an insertion point rather than the IRBuilder instance itself. I<br class="">had two<br class="">reasons to go this way:<br class=""><br class="">1. The insertion point can be adjusted manually in the IRBuilder.<br class="">This makes<br class="">it more obvious that the marker is associated with the original<br class="">insertion<br class="">point after a change.<br class=""><br class="">2. The behavior is more intuitive with nested IRBuilders.<br class=""><br class="">For example in the LV code that I am changing we should really move<br class="">the marker<br class="">earlier to also cover the SCEV expansion of the array bounds. Those<br class="">instructions are however inserted via an implicit IRBuilder inside<br class="">SCEV<br class="">expansion. The good thing is that the marker as written sees these<br class="">because of<br class="">the shared insertion point.<br class=""><br class="">The patch is still missing unit tests but wanted to float the idea<br class="">before I<br class="">get too carried away. Is this a reasonable idea? Are there other<br class="">issues I am<br class="">missing?<br class=""><br class=""><a href="http://reviews.llvm.org/D7421" class="">http://reviews.llvm.org/D7421</a><br class=""><br class="">Files:<br class=""> include/llvm/IR/IRBuilder.h<br class=""> lib/Transforms/Vectorize/LoopVectorize.cpp<br class=""><br class="">Index: include/llvm/IR/IRBuilder.h<br class="">===================================================================<br class="">--- include/llvm/IR/IRBuilder.h<br class="">+++ include/llvm/IR/IRBuilder.h<br class="">@@ -188,6 +188,24 @@<br class=""> /// \brief Set the fast-math flags to be used with generated<br class=""> fp-math operators<br class=""> void SetFastMathFlags(FastMathFlags NewFMF) { FMF = NewFMF; }<br class=""><br class="">+ class Marker {<br class="">+ BasicBlock::iterator Prev;<br class="">+ BasicBlock *BB;<br class="">+ public:<br class="">+ Marker(BasicBlock::iterator InsertionPoint) : BB(nullptr) {<br class="">+ if (InsertionPoint == InsertionPoint->getParent()->begin())<br class="">+ BB = InsertionPoint->getParent();<br class="">+ else<br class="">+ Prev = std::prev(InsertionPoint);<br class="">+ }<br class="">+ BasicBlock::iterator get() const {<br class="">+ if (BB)<br class="">+ return BB->begin();<br class="">+ else<br class="">+ return std::next(Prev);<br class="">+ }<br class="">+ };<br class="">+<br class=""> //===--------------------------------------------------------------------===//<br class=""> // RAII helpers.<br class=""> //===--------------------------------------------------------------------===//<br class="">Index: lib/Transforms/Vectorize/LoopVectorize.cpp<br class="">===================================================================<br class="">--- lib/Transforms/Vectorize/LoopVectorize.cpp<br class="">+++ lib/Transforms/Vectorize/LoopVectorize.cpp<br class="">@@ -2050,6 +2050,8 @@<br class=""> }<br class=""><br class=""> IRBuilder<> ChkBuilder(Loc);<br class="">+ IRBuilder<>::Marker BeforeInsertionMarker(Loc);<br class="">+<br class=""> // Our instructions might fold to a constant.<br class=""> Value *MemoryRuntimeCheck = nullptr;<br class=""> for (unsigned i = 0; i < NumPointers; ++i) {<br class="">@@ -2102,6 +2104,14 @@<br class=""> ConstantInt::getTrue(Ctx));<br class=""> ChkBuilder.Insert(Check, "memcheck.conflict");<br class=""> FirstInst = getFirstInst(FirstInst, Check, Loc);<br class="">+<br class="">+ Instruction *AnotherFirst = BeforeInsertionMarker.get();<br class="">+ assert(std::find_if(BasicBlock::iterator(AnotherFirst),<br class="">+ Loc->getParent()->end(),<br class="">+ [&](Instruction &I) {<br class="">+ return &I == FirstInst;<br class="">+ }) != Loc->getParent()->end());<br class="">+<br class=""> return std::make_pair(FirstInst, Check);<br class="">}<br class=""><br class="">EMAIL PREFERENCES<br class=""> http://reviews.llvm.org/settings/panel/emailpreferences/<br class=""><br class=""></blockquote><br style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;" class=""><span style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; float: none; display: inline !important;" class="">--<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;" class=""><span style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; float: none; display: inline !important;" class="">Hal Finkel</span><br style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;" class=""><span style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; float: none; display: inline !important;" class="">Assistant Computational Scientist</span><br style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;" class=""><span style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; float: none; display: inline !important;" class="">Leadership Computing Facility</span><br style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;" class=""><span style="font-family: Helvetica; font-size: 10px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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; float: none; display: inline !important;" class="">Argonne National Laboratory</span></div></blockquote></div><br class=""></body></html>