
<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></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 26, 2016, at 9:35 PM, Hal Finkel via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div style="font-style: normal; font-variant: normal; font-weight: normal; 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; font-family: arial, helvetica, sans-serif; font-size: 10pt;" class=""><br class="Apple-interchange-newline"><hr id="zwchr" class=""><blockquote style="border-left-width: 2px; border-left-style: solid; border-left-color: rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica, Arial, sans-serif; font-size: 12pt;" class=""><b class="">From:<span class="Apple-converted-space"> </span></b>"Chandler Carruth" <<a href="mailto:chandlerc@google.com" class="">chandlerc@google.com</a>><br class=""><b class="">To:<span class="Apple-converted-space"> </span></b>"Xinliang David Li" <<a href="mailto:xinliangli@gmail.com" class="">xinliangli@gmail.com</a>><br class=""><b class="">Cc:<span class="Apple-converted-space"> </span></b>"Hal Finkel" <<a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a>>, "llvm-dev" <<a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a>>, "Philip Reames" <<a href="mailto:listmail@philipreames.com" class="">listmail@philipreames.com</a>>, "Duncan P. N. Exon Smith" <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>>, "Sanjoy Das" <<a href="mailto:sanjoy@playingwithpointers.com" class="">sanjoy@playingwithpointers.com</a>><br class=""><b class="">Sent:<span class="Apple-converted-space"> </span></b>Friday, February 26, 2016 10:43:30 PM<br class=""><b class="">Subject:<span class="Apple-converted-space"> </span></b>Re: [llvm-dev] Possible soundness issue with available_externally (split from "RFC: Add guard intrinsics")<br class=""><br class=""><div dir="ltr" class=""><div class="gmail_quote"><div dir="ltr" class="">On Fri, Feb 26, 2016 at 8:40 PM Xinliang David Li <<a href="mailto:xinliangli@gmail.com" target="_blank" class="">xinliangli@gmail.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote">On Fri, Feb 26, 2016 at 8:30 PM, Chandler Carruth<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:chandlerc@google.com" target="_blank" class="">chandlerc@google.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div dir="ltr" class=""><div class="gmail_quote"><span class=""><div dir="ltr" class="">On Fri, Feb 26, 2016 at 8:15 PM Hal Finkel <<a href="mailto:hfinkel@anl.gov" target="_blank" class="">hfinkel@anl.gov</a>> wrote:</div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;"><div class=""><div style="font-family: arial, helvetica, sans-serif; font-size: 10pt;" class="">Can you explain your concerns about reliability? My idea is that we have a kind of delayed-name comdat section attribute that says that the name of the comdat section into which the function will be placed will be based on the hash of the contents (which should probably be the pre-isel IR-level contexts plus perhaps the other state that might affect backend optimization). Maybe just using the MI-level function contents will work too. It seems like this should be "perfect". I don't see the circumstances why function definitions optimized under identical optimization settings should differ in this sense.<br class=""></div></div></blockquote><div class=""><br class=""></div></span><div class="">Ok, let's consider Sanjoy's latest post.</div><div class=""><br class=""></div><div class="">I think we could end up with this routine in N different modules with slightly different amounts of information in each. Each of these in turn could cause us to deduce a slightly different set of attributes, and thus compute a different hash.</div><div class=""><br class=""></div><div class="">Now, certainly, there are a small number of variants here. Two, maybe three. But this could in the worst case cause us to have two copies of a *huge* number of functions.</div><div class=""><br class=""></div><div class="">My concern here is not that this is a *theoretically* hard problem to solve. I think there are good techniques to handle this. My concern is that it will be a substantial amount of work and require a reasonable amount of testing and experimentation.</div></div></div></blockquote><div class=""><br class=""></div></div></div></div><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class="">Such data won't be too hard to find out. It can be conservatively estimated by looking at the final binary code for each COMDAT to see how many 'clones's are needed due to different code gen -- the result is the upper bound as different code gen does not mean different deduced function attributes.</div><div class=""><br class=""></div><div class="">With PGO, the decision process will also become much simpler.</div></div></div></div></blockquote><div class=""><br class=""></div><div class="">Sure. And if you want to work on privatization in LLVM's IPO, that would be awesome.</div><div class=""><br class=""></div><div id="DWT15526" class="">But I think it is completely unreasonable to gate checking in code to test for the invalid transformation until this new optimization technique is available. We should be fixing the bug now, and then figuring out how to do privatization.</div></div></div></blockquote><br class="">In any case, please don't misunderstand. I'm not trying to gate bug fixes on significant new optimization features. I was (and am) suggesting that we not rush to just turning things off until we've given ourselves some time (at least a few days) to understand the extent of the problem and the options available for fixing it, and have a plan for doing so without introducing (or at least soon recovering from) significant regressions (if at all possible). Turning things off is always an easy (and available) solution, and the strength of this project is that we're generally hesitant to use it. This is a pretty fundamental design problem that was uncovered "by inspection", if this gets fixed tomorrow or two weeks from now is not a significant distinction. Every time I've pushed back on this the resulting discussion has been very productive (IMHO), and I'm glad I did. And, as I've stated, given that we do have solutions on the table that can fix the problem without overt performance regressions (i.e. aggressive privatization), we should prefer to use them unless they prove impractically complicated for short-term implementation.<br class=""><br class="">And I think this issue is worth a significant amount of discussion. Every C++ inline method, constructors, etc. all end up in comdat sections. To pick one example, for almost all of these, they take a "this" pointer, and we very commonly prove that we don't capture the "this" pointer. I suspect that giving up that alone is going to be a significant problem.<br class=""></div></div></blockquote><div><br class=""></div><div><br class=""></div><div>I'm coming back on this email, since you are concerned about the performance impact of this (and I think it is an important aspect). </div><div>I think it is worth mentioning that if you perform an LTO build (which you will do if you're serious about performance anyway), then *most* ODR functions will be internalized and you won't have any performance problem at all.</div><div><br class=""></div><div>-- </div><div>Mehdi</div><div> </div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div style="font-style: normal; font-variant: normal; font-weight: normal; 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; font-family: arial, helvetica, sans-serif; font-size: 10pt;" class=""><br class=""> -Hal<br class=""><blockquote style="border-left-width: 2px; border-left-style: solid; border-left-color: rgb(16, 16, 255); margin-left: 5px; padding-left: 5px; font-weight: normal; font-style: normal; text-decoration: none; font-family: Helvetica, Arial, sans-serif; font-size: 12pt;" class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""></div><div class=""><br class=""></div><div class="">-Chandler</div></div></div></blockquote><br class=""><br class=""><br class="">--<span class="Apple-converted-space"> </span><br class=""><div class=""><span name="x" class=""></span>Hal Finkel<br class="">Assistant Computational Scientist<br class="">Leadership Computing Facility<br class="">Argonne National Laboratory<span name="x" class=""></span><br class=""></div></div><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; 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; float: none; display: inline !important;" class="">_______________________________________________</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; 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;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; 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; float: none; display: inline !important;" class="">LLVM Developers mailing list</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; 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;" class=""><a href="mailto:llvm-dev@lists.llvm.org" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; 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;" class="">llvm-dev@lists.llvm.org</a><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; 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;" class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; 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;" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a></div></blockquote></div><br class=""></body></html>