<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 22, 2017 at 7:46 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><span class=""><div dir="ltr">On Tue, Aug 22, 2017 at 7:18 PM Xinliang David Li via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Aug 22, 2017 at 7:10 PM, Chandler Carruth via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span><div dir="ltr">On Tue, Aug 22, 2017 at 7:03 PM Xinliang David Li via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Aug 22, 2017 at 6:37 PM, Chandler Carruth via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">chandlerc added a comment.<br>
<br>
I'm really not a fan of the degree of complexity and subtlety that this introduces into the frontend, all to allow particular backend optimizations.<br>
<br>
I feel like this is Clang working around a fundamental deficiency in LLVM and we should instead find a way to fix this in LLVM itself.<br>
<br>
As has been pointed out before, user code can synthesize large integers that small bit sequences are extracted from, and Clang and LLVM should handle those just as well as actual bitfields.<br>
<br>
Can we see how far we can push the LLVM side before we add complexity to Clang here? I understand that there remain challenges to LLVM's stuff, but I don't think those challenges make *all* of the LLVM improvements off the table, I don't think we've exhausted all ways of improving the LLVM changes being proposed, and I think we should still land all of those and re-evaluate how important these issues are when all of that is in place.<br></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>The main challenge of doing  this in LLVM is that inter-procedural analysis (and possibly cross module) is needed (for store forwarding issues).</div><div><br></div><div>Wei, perhaps you can provide concrete test case to illustrate the issue so that reviewers have a good understanding.</div></div></div></div></blockquote><div><br></div></span><div>It doesn't seem like all options for addressing that have been exhausted. And even then, I feel like trying to fix this with non-obvious (to the programmer) frontend heuristics isn't a good solution. I actually *prefer* the source work around of "don't use a bitfield if you *must* have narrow width access across modules where the optimizer cannot see enough to narrow them and you happen to know that there is a legal narrow access that works". Because that way the programmer has *control* over this rather than being at the whim of whichever side of the heuristic they end up on.</div></div></div></blockquote><div><br></div><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>The source workaround solution *does not* scale. Most importantly, user may not even be aware of the problem (and performance loss) unless  compiling the code with another compiler and notice the performance difference.</div></div></div></div></blockquote><div><br></div></span><div>I don't really understand this argument...</div><div><br></div><div>I don't think we can realistically chase all performance problems that users aren't aware of or can't measure. And our users *do* care and seem very good at benchmarking and noticing problems. =]</div><div><br></div></div></div></blockquote><div><br></div><div>Here you assume that there is single hotspot that user can notice easily, but this is usually not the case. In a very large scale, we have a long tail of not so hot functions, but similar problems like this can add up.</div><div><br></div><div>Besides if the problem is obvious for user to detect, why wasn't this problem detected in LLVM earlier?   I admit, in this particular case, we actually relied on the competitive analysis -- that is why I said depending on users for this is not reliable.</div><div><br></div><div>For yearsI have told our users (optimization) not to hack source code/or do manual performance tuning (for instance, adding always inlining, adding inline keyword, adding manually inserted prefetchings, manual loop unrolling/peelings, etc ). Not only does this method does not scale (it only helps user's own program), but more importantly it may get stale overtime -- for instance, due to hardware change or shifting of hot spots. It is not uncommon to see user's manual optimiztion which used to work later becomes performance 'bug' without being noticed for years.   I am totally fine with temporary source workaround, but they should not relied upon longer term.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div></div><div>I also think it is OK if in rare cases programmers have to adapt their source code to optimize well. We can and should make this as rare as reasonable from an engineering tradeoff perspective, but at a certain point we need to work with programmers to find a way to solve the problem. There exist coding patterns that Clang and LLVM will never be particularly effective at optimizing, and that seems OK. We just need to have reasonable engineering requirements for those cases:</div></div></div></blockquote><div><br></div><div>As I said, for rare cases or as a temporary solution, it is fine to use source workaround, but we should continue to push for compiler based solution because compiler is for everybody/every app.  If compiler really sucks at something, it should at least produce warnings about (with something like performance sanitizer), but that should be used as last resort.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div>1) They need to be quite rare. So far, this issue has come up fairly infrequently. Not never, but there are many issues that seem to come up much more often such as inlining issues.</div><div><br></div><div>2) We need an effective way to explain how programmers should write code to optimize well, and this explanation needs to be as simple as possible.</div><div><br></div><div>3) We likely need to give programmers *choices* about which direction their code gets optimized for. And this is (IMO) the biggest practical issue with the approach in this change (beyond race detection and other semantic issues).</div><div><br></div><div><br></div><div>I'm suggesting that we teach programmers to choose between a bitfield to get combining-friendly access to tightly packed data, and integer types of an appropriate size to get inexpensive and predictable loads and stores. For sub-byte-width and non-byte-aligned bitfields this is already a necessary property and so it seems like a consistent and easily explained and taught model.</div><div><br></div><div>While there are times where there is a single "right" answer and we only have to provide that, I don't think this is one of them. I've seen rare but serious complaints about *both* failing to combine adjacent bit fields and failing to narrow to small legal integer types. These seem in fundamental tension, and I would prefer to establish idiomatic patterns for programmers to request the behavior then need in their application.</div></div></div></blockquote><div><br></div><div>If we can teach programmers how to to make a good choice, why can't we teach the compiler to do so?  I don't  think it is a good idea to set up a precedence to give up easily on engineering solutions in compiler.</div><div><br></div><div>David</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span class=""><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>David</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>David</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="m_7521289160219057262m_-7067370907494956458m_-8403912814972070661m_5508161617453186913HOEnZb"><div class="m_7521289160219057262m_-7067370907494956458m_-8403912814972070661m_5508161617453186913h5"><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D36562" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D36562</a><br>
<br>
<br>
<br>
</div></div></blockquote></div></div></div></span>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div>
<br>______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div></div></div>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></span></div></div>
</blockquote></div><br></div></div>