<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=""><div class="">Hi,</div><div class=""><br class=""></div>DAG combiner does indeed optimizations but also canonicalization.<div class="">The latter, we probably need to keep, the former, we should be able to disable.</div><div class=""><br class=""></div><div class="">When I asked Marianne to do a RFC about this, I was hoping we could get new ideas on how to tackle this problem.</div><div class=""><br class=""></div><div class="">I am fine with the approach of disabling the optimizations one by one when we know it is indeed an optimization. However, I am not a fan of having "if (OptLevel == <...>)" scattered all around the place.</div><div class="">Instead, I would like to have a sort of list of optimizations that differ between the opt level == none and the other opt level, then we run just that list.</div><div class=""><br class=""></div><div class="">In other words, I was thinking we could have some kind of registration mechanism for each optimization and this is a matter of classifying them.</div><div class=""><br class=""></div><div class="">Anyhow, what are other people thoughts?</div><div class=""><br class=""></div><div class="">Note: I understand this is a big refactoring and maybe not one that we want to do now, but I believe we could reuse some of the ideas we are discussing here when we eventually do combines in GobalISel.</div><div class=""><br class=""></div><div class="">Cheers,</div><div class="">-Quentin</div><div class=""><br class=""><div class=""><div><blockquote type="cite" class=""><div class="">On May 13, 2016, at 12:51 PM, Robinson, Paul 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 class="WordSection1" style="page: WordSection1; font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: 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;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">(Remembering to re-add llvm-dev again…)<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">Yes, this would be too aggressive.  DAG combiner does more than just optimization (which I discovered as part of D7181, see PR22346).<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">Internally we've thrown around other ideas, for example inside the worklist loop, skip loads and stores at O0, but haven't done any actual experiments.  There are probably other bits of the combiner that could be turned off at O0, but I think they'd have to be considered individually.  If you want to run with this one, go right ahead.<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">--paulr<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class="">Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class="">===================================================================<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class="">--- lib/CodeGen/SelectionDAG/DAGCombiner.cpp     (revision 269398)<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class="">+++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp     (working copy)<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class="">@@ -1300,6 +1300,10 @@<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class="">         continue;<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class="">     }<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class="">+    if (OptLevel == CodeGenOpt::None &&<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class="">+        (N->getOpcode() == ISD::LOAD || N->getOpcode() == ISD::STORE))<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class="">+      continue;<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class="">+<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class="">     DEBUG(dbgs() << "\nCombining: "; N->dump(&DAG));<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: 'Courier New'; color: rgb(31, 73, 125);" class="">     // Add any operands of the new node which have not yet been combined to the<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><a name="_MailEndCompose" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></a></div><div style="border-style: none none none solid; border-left-color: blue; border-left-width: 1.5pt; padding: 0in 0in 0in 4pt;" class=""><div class=""><div style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0in 0in;" class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><b class=""><span style="font-size: 10pt; font-family: Tahoma, sans-serif;" class="">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif;" class=""><span class="Apple-converted-space"> </span>llvm-dev [<a href="mailto:llvm-dev-bounces@lists.llvm.org" class="">mailto:llvm-dev-bounces@lists.llvm.org</a>]<span class="Apple-converted-space"> </span><b class="">On Behalf Of<span class="Apple-converted-space"> </span></b>Marianne Mailhot-Sarrasin via llvm-dev<br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>Friday, May 13, 2016 12:21 PM<br class=""><b class="">To:</b><span class="Apple-converted-space"> </span><a href="mailto:llvm-dev@lists.llvm.org" class="">llvm-dev@lists.llvm.org</a><br class=""><b class="">Subject:</b><span class="Apple-converted-space"> </span>[llvm-dev] [RFC] Disabling DAG combines in /O0<o:p class=""></o:p></span></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Hi all,<o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">The DAGCombiner pass actually runs even if the optimize level is set to None. This can result in incorrect debug information or unexpected stepping/debugging experience. Not to mention that having good stepping/debugging experience is the major reason to compile at /O0.<o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">I recently suggested a patch to disable one specific DAG combine at /O0 that broke stepping on a particular case (<a href="http://reviews.llvm.org/D19268" style="color: purple; text-decoration: underline;" class="">http://reviews.llvm.org/D19268</a>), but other similar cases could appear. In the same way, another patch was submitted last year for a similar reason (<a href="http://reviews.llvm.org/D7181" style="color: purple; text-decoration: underline;" class="">http://reviews.llvm.org/D7181</a>).<o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">So, since the DAGCombiner is in fact an optimization pass, could it be disabled completely (or partially?) in /O0? And how should it be done?<o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">For example, would this patch be too aggressive?<o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><br class=""><span style="font-size: 10pt;" class="">Index: DAGCombiner.cpp<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class="">===================================================================<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class="">--- DAGCombiner.cpp        (revision 269301)<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class="">+++ DAGCombiner.cpp     (working copy)<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class="">@@ -1251,6 +1251,10 @@<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class="">//===----------------------------------------------------------------------===//<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class=""> void DAGCombiner::Run(CombineLevel AtLevel) {<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class="">+<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class="">+  if (OptLevel == CodeGenOpt::None)<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class="">+    return;<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class="">+<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class="">   // set the instance variables, so that the various visit routines may use it.<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class="">   Level = AtLevel;<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 1in; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 10pt;" class="">   LegalOperations = Level >= AfterLegalizeVectorOps;<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">It would most likely break some CodeGen tests since it would have an impact on the code produced at /O0. In fact, I tried to run the CodeGen lit tests with this patch and got 25 new test failures on different targets. These tests would probably need to be updated.<o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Thanks,<o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Marianne<o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif;" class=""><o:p class=""> </o:p></span></div></div></div><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: 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-caps: 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-caps: 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-caps: 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="color: purple; text-decoration: underline; font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: 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-caps: 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="color: purple; text-decoration: underline; font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: 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=""></div></div></body></html>