<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 Mar 9, 2015, at 3:03 PM, Eric Christopher <<a href="mailto:echristo@gmail.com" class="">echristo@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><br class=""><div class="gmail_quote">On Mon, Mar 9, 2015 at 2:49 PM Mehdi AMINI <<a href="mailto:mehdi.amini@apple.com" class="">mehdi.amini@apple.com</a>> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thanks for review, that was quick :)<br class="">
<br class="">
<br class=""></blockquote><div class=""><br class=""></div><div class="">Sure :)</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
REPOSITORY<br class="">
rL LLVM<br class="">
<br class="">
================<br class="">
Comment at: lib/Analysis/Lint.cpp:178<br class="">
@@ -177,3 +176,2 @@<br class="">
DT = &getAnalysis<<u class=""></u>DominatorTreeWrapperPass>().<u class=""></u>getDomTree();<br class="">
- DL = &F.getParent()->getDataLayout(<u class=""></u>);<br class="">
TLI = &getAnalysis<<u class=""></u>TargetLibraryInfoWrapperPass>(<u class=""></u>).getTLI();<br class="">
----------------<br class="">
echristo wrote:<br class="">
> Any reason not to leave it cached instead of passing it down?<br class="">
In short: cannot rebind a reference and I wanted to get rid of the pointer.<br class="">
<br class=""></blockquote><div class=""><br class=""></div><div class=""><span style="font-size:13.1999998092651px" class="">Ah, sure.</span></div><div class=""><span style="font-size:13.1999998092651px" class=""> </span><br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
It plays well with passes that initialize a separate transient object to do the processing, but not with passes that do their processing "in-place". Because passes object are not transient, they carry their state between runs, including a potential obsolete DL pointer.<br class=""></blockquote><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Not sure if I was clear?<br class="">
<br class=""></blockquote><div class=""><br class=""></div><div class="">Assuming in the case of "things that work on multiple modules". Otherwise, something that just runs on a single module should be able to cache it with no problem right? :)</div></div></div></div></blockquote><div><br class=""></div><div>Even in the case “work on multiple modules” it should be fine. The problem is to be sure that all the paths that can call a function that used the cached DL are coming from the entry point of the pass execution (runOnFunction() and cie), which is supposed to update the cached pointer. </div><div>This makes the code a bit harder to analyze compared to transient object.</div><div>Note that on the opposite, I added a cached DataLayout sometimes where it was possible in order to avoid passing an extra argument (for instance SCEVExpander).</div><div><br class=""></div><div>I have a cleanup update coming soon.</div><div><br class=""></div><div> — </div><div>Mehdi</div><div><br class=""></div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This was very case-by-case, and I may have get it wrong here and there (I mean wrong in the sense that a function argument could have been save by retrieving the DL differently, I don't think I broke correctness).<br class="">
<br class=""></blockquote><div class=""><br class=""></div><div class="">Right. Should be ok.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
================<br class="">
Comment at: lib/Analysis/<u class=""></u>LoopAccessAnalysis.cpp:292<br class="">
@@ -291,2 +291,3 @@<br class="">
if (hasComputableBounds(SE, StridesMap, Ptr) &&<br class="">
- // When we run after a failing dependency check we have to make sure we<br class="">
+ // When we run after a failing dependency check we have to make sure<br class="">
+ // we<br class="">
----------------<br class="">
echristo wrote:<br class="">
> Formatting. :)<br class="">
That was clang-format, it probably gets over 80 lines without the line break. I'll rephrase the comment to save what is needed to fit in the line.<br class="">
<br class=""></blockquote><div class=""><br class=""></div><div class="">Yep. Figured, it does that to me all the time, just calling it out.</div><div class=""><br class=""></div><div class="">-eric</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<a href="http://reviews.llvm.org/D8158" target="_blank" class="">http://reviews.llvm.org/D8158</a><br class="">
<br class="">
EMAIL PREFERENCES<br class="">
<a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank" class="">http://reviews.llvm.org/<u class=""></u>settings/panel/<u class=""></u>emailpreferences/</a><br class="">
<br class="">
<br class="">
</blockquote></div></div>
</div></blockquote></div><br class=""></body></html>