<div dir="ltr">IMHO, we should delete the comparison operators, or make STATISTIC work in release mode.<div>(I have trouble thinking they are so slow that it's worth compiling them out)</div><div><br><div><br></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, May 8, 2017 at 11:36 AM, Craig Topper 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 dir="ltr"><div>We recently found a bug in an internal pass where we compared a Statistic object against another value that resulted in a mismatch in output between debug and release builds. This is because Statistic objects are designed to be compiled out of release builds. So their increment operator doesn't do anything and any attempt to read its value returns 0 in release builds.</div><div><br></div><div>Another thing to note is that Statistic objects are global variables that count across multiple pass runs and never reset.</div><div><br></div><div>After noticing this behavior we did an audit of trunk to see if we could find any cases of similar mistakes. We did this by marking all comparison operators on the Statistic object as deleted. In total we found 6 files in trunk that failed to build after this.</div><div><br></div><div>lib/Target/ARM/<wbr>ARMOptimizeBarriersPass.cpp had a return value from runOnMachineFunction dependent on a compare of a Statistic object with 0. This would always return false in a release build, and always return true for any pass run after the first pass run that incremented the statistic. I've fixed this one in r302450 as it was definitely the most suspicious.</div><div><br></div><div>lib/CodeGen/TailDuplicator.<wbr>cpp:257 has a compare between a statistic and a command line option. I think this makes the command line option broken in release builds. In debug builds, probably is also applying the command line option commulatively across each function the pass runs on.</div><div><br></div><div>lib/Transforms/<wbr>Instrumentation/<wbr>IndirectCallPromotion.cpp:339,<wbr>359,730 have more compares between command line options and statistics.</div><div><br></div><div>lib/Target/ARM/<wbr>MLxExpansionPass.cpp:214 has another compare between statistic and command line argument.</div><div><br></div><div>The last 3 don't appear to be a functional problem:</div><div><br></div><div>lib/Analysis/CallGraphSCCPass.<wbr>cpp:481 has a compare in order to update a "max" statistic.</div><div><br></div><div>tools/clang/lib/<wbr>StaticAnalysis/Frontend/<wbr>AnalysisConsumer.cpp:679 has a compare for a max statistic.</div><div><br></div><div>tools/clang/lib/<wbr>StaticAnalysis/Frontend/<wbr>AnalysisConsumer.cpp:568 has a compare to avoid a divide by 0 in calculating another statistic.</div><div><br></div><div><br></div><div>Should we do something about the first set of cases? Should we do something to prevent creating more such issues in the future by deleting the comparison operators and providing hooks specifically for the 3 ok cases?</div><span class="HOEnZb"><font color="#888888"><br clear="all"><div><div class="m_3880271483595180362gmail_signature">~Craig</div></div>
</font></span></div>
<br>______________________________<wbr>_________________<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/<wbr>mailman/listinfo/llvm-dev</a><br>
<br></blockquote></div><br></div>