<div dir="ltr"><div>Hi Marianne,</div><div><br></div>This has been tried before. The problem is that although, ideally, DAGCombine should only be used for optimization, in practice there exist some combines that are required for correctness. So making this work properly may require a fairly large clean-up job and a lot of testing.<div>I suggest you look at PR22346, <a href="http://reviews.llvm.org/D8614">http://reviews.llvm.org/D8614</a> and <a href="http://reviews.llvm.org/D9992">http://reviews.llvm.org/D9992</a> for context.</div><div><br></div><div>Michael</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 13, 2016 at 12:21 PM, Marianne Mailhot-Sarrasin via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@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 lang="EN-US" link="blue" vlink="purple">
<div>
<p class="MsoNormal">Hi all,<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">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.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">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" target="_blank">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" target="_blank">http://reviews.llvm.org/D7181</a>).<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">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?<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">For example, would this patch be too aggressive?<u></u><u></u></p>
<p class="MsoNormal" style="margin-left:1.0in"><br>
<span style="font-size:10.0pt">Index: DAGCombiner.cpp<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt">===================================================================<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt">--- DAGCombiner.cpp (revision 269301)<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt">+++ DAGCombiner.cpp (working copy)<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt">@@ -1251,6 +1251,10 @@<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt">//===----------------------------------------------------------------------===//<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt"><u></u> <u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt"> void DAGCombiner::Run(CombineLevel AtLevel) {<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt">+<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt">+ if (OptLevel == CodeGenOpt::None)<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt">+ return;<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt">+<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt"> // set the instance variables, so that the various visit routines may use it.<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt"> Level = AtLevel;<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:1.0in"><span style="font-size:10.0pt"> LegalOperations = Level >= AfterLegalizeVectorOps;<u></u><u></u></span></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">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.<u></u><u></u></p>
<p class="MsoNormal"><u></u> <u></u></p>
<p class="MsoNormal">Thanks,<u></u><u></u></p>
<p class="MsoNormal">Marianne<u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><u></u> <u></u></span></p>
</div>
</div>
<br>_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
<br></blockquote></div><br></div>