<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 5, 2015, at 1:06 AM, 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: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>><br class="">Cc:<span class="Apple-converted-space"> </span><a href="mailto:reviews+D7421+public+86df0334cf13545b@reviews.llvm.org" class="">reviews+D7421+public+86df0334cf13545b@reviews.llvm.org</a>, "llvm-commits" <<a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a>>, "Nadav<br class="">Rotem(<a href="mailto:nrotem@apple.com" class="">nrotem@apple.com</a>)" <<a href="mailto:nrotem@apple.com" class="">nrotem@apple.com</a>>, "Arnold Schwaighofer" <<a href="mailto:aschwaighofer@apple.com" class="">aschwaighofer@apple.com</a>>,<span class="Apple-converted-space"> </span><a href="mailto:chandlerc@gmail.com" class="">chandlerc@gmail.com</a><br class="">Sent: Thursday, February 5, 2015 2:19:24 AM<br class="">Subject: Re: [PATCH] IRBuilder: Allow retrieval of the inserted instructions<br class=""><br class="">On Feb 4, 2015, at 10:15 PM, Hal Finkel <<span class="Apple-converted-space"> </span><a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a><span class="Apple-converted-space"> </span>> wrote:<br class=""><br class=""><br class="">----- Original Message -----<br class=""><br class=""><br class="">From: "Adam Nemet" <<span class="Apple-converted-space"> </span><a href="mailto:anemet@apple.com" class="">anemet@apple.com</a><span class="Apple-converted-space"> </span>><br class="">To: "Hal Finkel" <<span class="Apple-converted-space"> </span><a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a><span class="Apple-converted-space"> </span>><br class="">Cc:<span class="Apple-converted-space"> </span><a href="mailto:reviews+D7421+public+86df0334cf13545b@reviews.llvm.org" class="">reviews+D7421+public+86df0334cf13545b@reviews.llvm.org</a><span class="Apple-converted-space"> </span>,<br class="">"llvm-commits" <<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><span class="Apple-converted-space"> </span>>, "Nadav Rotem <<br class=""><a href="mailto:nrotem@apple.com" class="">nrotem@apple.com</a><span class="Apple-converted-space"> </span>> (<span class="Apple-converted-space"> </span><a href="mailto:nrotem@apple.com" class="">nrotem@apple.com</a><span class="Apple-converted-space"> </span>)" <<span class="Apple-converted-space"> </span><a href="mailto:nrotem@apple.com" class="">nrotem@apple.com</a><span class="Apple-converted-space"> </span>>,<br class="">"Arnold Schwaighofer" <<span class="Apple-converted-space"> </span><a href="mailto:aschwaighofer@apple.com" class="">aschwaighofer@apple.com</a><span class="Apple-converted-space"> </span>>,<br class=""><a href="mailto:chandlerc@gmail.com" class="">chandlerc@gmail.com</a><br class="">Sent: Wednesday, February 4, 2015 10:04:54 PM<br class="">Subject: Re: [PATCH] IRBuilder: Allow retrieval of the inserted<br class="">instructions<br class=""><br class="">On Feb 4, 2015, at 6:14 PM, Hal Finkel <<span class="Apple-converted-space"> </span><a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a><span class="Apple-converted-space"> </span>> wrote:<br class=""><br class="">----- Original Message -----<br class=""><br 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> ,<br class=""><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<br class="">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=""><br class="">Silly question: Why don't we split the block first?<br class=""><br class=""><br class=""><br class="">Because we may end up not needing any checks in which case we don’t<br class="">split. The decision of whether checks are necessary is local to both<br class="">addRuntimeCheck and addStrideCheck.<br class="">Okay, but an alternative design would be to always split early, and<br class="">then let SimplifyCFG put them back together again should we not<br class="">actually end up with any checks, right? I'm not necessarily<br class="">advocating for this approach, and think that the marker code will be<br class="">cleaner than the existing getFirstInst code, but splitting the block<br class="">eagerly will also create a trivial way of marking that point, and I<br class="">want to understand if there are any downsides to that approach<br class="">before we invent something new.<br class=""><br class=""><br class=""><br class="">I am not aware of any other downsides but I haven’t written this code<br class="">of course. Hopefully Nadav/Arnold will chime in if there is<br class="">something to add here.<br class=""><br class=""><br class="">Perhaps as another argument you could consider that there seems to be<br class="">some existing practice to fix up simple things like this in the pass<br class="">itself rather than relying on other passes. Especially in this case<br class="">where we could avoid producing the potential inefficiency.<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="">You're certainly right about that. And I certainly have no problems with this so long as it remains embedded inside the transformation itself.</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="">I guess I am also interested if you see some gotchas with the marker<br class="">feature or you just think that it’s not a generally useful new<br class="">addition.<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="">I'm not sure that I'm seeing the complete picture. Are these going to be on the new interface boundary between the vectorization analysis utilities and their users? Or just used internally?</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>getFirstInst is used by both addRuntimeCheck and addStrideCheck. I’d like to move addRuntimeCheck to LAA but not stride checking. Stride checking has some more dependencies that makes this a bit more involved. So getFirstInst or the corresponding feature would have to live somewhere where it could be shared between to LV.cpp and LAA.cpp.</div><div><br class=""></div><div>I hope this clears up the picture.</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="">Any why is this marker class associated with IRBuilder?</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>I doesn’t have to. It’s actually associated with an insertion point. I put it in the IRBuilder header because:</div><div><br class=""></div><div>1. I felt that insertion point was an IRBuilder concept</div><div>2. Its main aim is to query the result of IRBuilder inserts that may or may not end up generating instructions</div><div>3. I was thinking that it’s a generally useful feature essentially answering the question of what IRBuilder has actually done with the instruction stream through its various levels of smartness</div><div><br class=""></div><div>We can obviously move it under LAA or as you say rip it out completely and have SimplifyCFG fix this up for us. I was mostly trying to test the waters if this was a generally interesting addition.</div><div><br class=""></div><div>Sounds like it’s not. I can try to see if there are code changes without getFirstInst and report back.</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="">Thanks again,</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</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=""><br class="">Thanks,<br class="">Adam<br class=""><br class="">Thanks again,<br class="">Hal<br class=""><br class="">Adam<br class=""><br class=""><br class=""><br class="">-Hal<br 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="">--<br class=""><br class="">Hal Finkel<br class="">Assistant Computational Scientist<br class="">Leadership Computing Facility<br class="">Argonne National Laboratory<br class=""><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>