<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=""><blockquote type="cite" class="">hfinkel: Are you going to separate this into a follow-up patch, or are you planning to provide compile-time measurements to before committing this one?</blockquote></div>I guess I had said that compile-time is not impacted, but never showed any data. And then I had said many more things about compile-time measurements, our test suite and dominators, so the noise numbers should come as no surprise (all with 0.1-0.2 secs on x86):<div class=""><br class=""></div><div class=""><table class="sortable" style="font-family: Helvetica, sans-serif; font-size: 9pt; border-spacing: 0px; border: 1px solid black;"><tbody class=""><tr class=""><th width="500" style="background-color: rgb(238, 238, 238); color: rgb(102, 102, 102); cursor: default; text-align: center; font-family: Verdana; padding: 5px 5px 5px 8px;" class="">Performance Regressions - Compile Time</th><th style="background-color: rgb(238, 238, 238); color: rgb(102, 102, 102); cursor: default; text-align: center; font-family: Verdana; padding: 5px 5px 5px 8px;" class="">Δ </th><th style="background-color: rgb(238, 238, 238); color: rgb(102, 102, 102); cursor: default; text-align: center; font-family: Verdana; padding: 5px 5px 5px 8px;" class="">Previous</th><th style="background-color: rgb(238, 238, 238); color: rgb(102, 102, 102); cursor: default; text-align: center; font-family: Verdana; padding: 5px 5px 5px 8px;" class="">Current</th><th style="background-color: rgb(238, 238, 238); color: rgb(102, 102, 102); cursor: default; text-align: center; font-family: Verdana; padding: 5px 5px 5px 8px;" class="">σ </th><th style="background-color: rgb(238, 238, 238); color: rgb(102, 102, 102); cursor: default; text-align: center; font-family: Verdana; padding: 5px 5px 5px 8px;" class="">Δ (B)</th><th style="background-color: rgb(238, 238, 238); color: rgb(102, 102, 102); cursor: default; text-align: center; font-family: Verdana; padding: 5px 5px 5px 8px;" class="">σ (B)</th></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.202=2" class="">SingleSource/UnitTests/ObjC++/property-reference</a></td><td bgcolor="#ffa6a6" style="padding: 5px 5px 5px 8px;" class="">4.37%</td><td style="padding: 5px 5px 5px 8px;" class="">0.4192</td><td style="padding: 5px 5px 5px 8px;" class="">0.4375</td><td style="padding: 5px 5px 5px 8px;" class="">0.0003</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0003</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.255=2" class="">SingleSource/Benchmarks/Shootout-C++/fibo</a></td><td bgcolor="#ffaaaa" style="padding: 5px 5px 5px 8px;" class="">3.98%</td><td style="padding: 5px 5px 5px 8px;" class="">0.2714</td><td style="padding: 5px 5px 5px 8px;" class="">0.2822</td><td style="padding: 5px 5px 5px 8px;" class="">0.0005</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0005</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.521=2" class="">MultiSource/Applications/lemon/lemon</a></td><td bgcolor="#ffb1b1" style="padding: 5px 5px 5px 8px;" class="">3.35%</td><td style="padding: 5px 5px 5px 8px;" class="">0.5018</td><td style="padding: 5px 5px 5px 8px;" class="">0.5186</td><td style="padding: 5px 5px 5px 8px;" class="">0.0045</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0045</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.62=2" class="">SingleSource/UnitTests/ObjC++/property-reference-object</a></td><td bgcolor="#ffb4b4" style="padding: 5px 5px 5px 8px;" class="">3.05%</td><td style="padding: 5px 5px 5px 8px;" class="">0.3375</td><td style="padding: 5px 5px 5px 8px;" class="">0.3478</td><td style="padding: 5px 5px 5px 8px;" class="">0.0002</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0002</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.513=2" class="">MultiSource/Applications/sqlite3/sqlite3</a></td><td bgcolor="#ffb7b7" style="padding: 5px 5px 5px 8px;" class="">2.81%</td><td style="padding: 5px 5px 5px 8px;" class="">7.1620</td><td style="padding: 5px 5px 5px 8px;" class="">7.3631</td><td style="padding: 5px 5px 5px 8px;" class="">0.0438</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0438</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.163=2" class="">External/SPEC/CFP2006/444_namd/444_namd</a></td><td bgcolor="#ffbfbf" style="padding: 5px 5px 5px 8px;" class="">2.15%</td><td style="padding: 5px 5px 5px 8px;" class="">3.1007</td><td style="padding: 5px 5px 5px 8px;" class="">3.1675</td><td style="padding: 5px 5px 5px 8px;" class="">0.0021</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0021</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.537=2" class="">MultiSource/Applications/oggenc/oggenc</a></td><td bgcolor="#ffc5c5" style="padding: 5px 5px 5px 8px;" class="">1.73%</td><td style="padding: 5px 5px 5px 8px;" class="">2.0891</td><td style="padding: 5px 5px 5px 8px;" class="">2.1252</td><td style="padding: 5px 5px 5px 8px;" class="">0.0065</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0065</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.447=2" class="">External/SPEC/CINT2000/197_parser/197_parser</a></td><td bgcolor="#ffc7c7" style="padding: 5px 5px 5px 8px;" class="">1.63%</td><td style="padding: 5px 5px 5px 8px;" class="">1.1629</td><td style="padding: 5px 5px 5px 8px;" class="">1.1819</td><td style="padding: 5px 5px 5px 8px;" class="">0.0000</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0000</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.333=2" class="">External/SPEC/CINT2000/176_gcc/176_gcc</a></td><td bgcolor="#ffc7c7" style="padding: 5px 5px 5px 8px;" class="">1.60%</td><td style="padding: 5px 5px 5px 8px;" class="">12.4641</td><td style="padding: 5px 5px 5px 8px;" class="">12.6631</td><td style="padding: 5px 5px 5px 8px;" class="">0.0030</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0030</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.528=2" class="">External/SPEC/CINT95/099_go/099_go</a></td><td bgcolor="#ffc9c9" style="padding: 5px 5px 5px 8px;" class="">1.51%</td><td style="padding: 5px 5px 5px 8px;" class="">1.7386</td><td style="padding: 5px 5px 5px 8px;" class="">1.7649</td><td style="padding: 5px 5px 5px 8px;" class="">0.0020</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0020</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.59=2" class="">External/SPEC/CINT2006/403_gcc/403_gcc</a></td><td bgcolor="#ffcaca" style="padding: 5px 5px 5px 8px;" class="">1.46%</td><td style="padding: 5px 5px 5px 8px;" class="">30.0626</td><td style="padding: 5px 5px 5px 8px;" class="">30.5019</td><td style="padding: 5px 5px 5px 8px;" class="">0.0195</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0195</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.319=2" class="">External/SPEC/CINT95/134_perl/134_perl</a></td><td bgcolor="#ffcccc" style="padding: 5px 5px 5px 8px;" class="">1.34%</td><td style="padding: 5px 5px 5px 8px;" class="">2.5077</td><td style="padding: 5px 5px 5px 8px;" class="">2.5413</td><td style="padding: 5px 5px 5px 8px;" class="">0.0010</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0010</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.280=2" class="">External/SPEC/CINT2006/400_perlbench/400_perlbench</a></td><td bgcolor="#ffcccc" style="padding: 5px 5px 5px 8px;" class="">1.31%</td><td style="padding: 5px 5px 5px 8px;" class="">10.8686</td><td style="padding: 5px 5px 5px 8px;" class="">11.0114</td><td style="padding: 5px 5px 5px 8px;" class="">0.0005</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0005</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.272=2" class="">External/SPEC/CINT2006/445_gobmk/445_gobmk</a></td><td bgcolor="#ffcfcf" style="padding: 5px 5px 5px 8px;" class="">1.18%</td><td style="padding: 5px 5px 5px 8px;" class="">7.7186</td><td style="padding: 5px 5px 5px 8px;" class="">7.8093</td><td style="padding: 5px 5px 5px 8px;" class="">0.0010</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0010</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.241=2" class="">External/SPEC/CINT2000/253_perlbmk/253_perlbmk</a></td><td bgcolor="#ffd0d0" style="padding: 5px 5px 5px 8px;" class="">1.11%</td><td style="padding: 5px 5px 5px 8px;" class="">5.5141</td><td style="padding: 5px 5px 5px 8px;" class="">5.5752</td><td style="padding: 5px 5px 5px 8px;" class="">0.0047</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0047</td></tr><tr class=""><td style="padding: 5px 5px 5px 8px;" class=""><a href="http://ghoflehner.apple.com/perf/v4/nts/2/graph?test.99=2" class="">External/SPEC/CINT2000/254_gap/254_gap</a></td><td bgcolor="#ffd1d1" style="padding: 5px 5px 5px 8px;" class="">1.03%</td><td style="padding: 5px 5px 5px 8px;" class="">4.5858</td><td style="padding: 5px 5px 5px 8px;" class="">4.6332</td><td style="padding: 5px 5px 5px 8px;" class="">0.0058</td><td bgcolor="#ffffff" style="padding: 5px 5px 5px 8px;" class="">0.00%</td><td style="padding: 5px 5px 5px 8px;" class="">0.0058</td></tr></tbody></table><div class=""><br class=""></div><div><blockquote type="cite" class=""><div class="">On Sep 28, 2014, at 9:00 AM, <a href="mailto:hfinkel@anl.gov" class="">hfinkel@anl.gov</a> wrote:</div><br class="Apple-interchange-newline"><div class="">================<br class="">Comment at: lib/Transforms/InstCombine/InstCombine.h:247<br class="">@@ -244,2 +246,3 @@<br class=""> Instruction *visitInstruction(Instruction &I) { return nullptr; }<br class="">+ bool dominatesAllUses(const Instruction *DI, const Instruction *UI, const BasicBlock *DB) const;<br class=""><br class="">----------------<br class="">80 cols.<br class=""><br class="">================<br class="">Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2428<br class="">@@ -2427,1 +2427,3 @@<br class=""><br class="">+/// \brief Check that one use is in the same block as the defintion and all<br class="">+/// other uses are in blocks dominated by a given block<br class="">----------------<br class="">This is not quite right, the function actually asserts if the definition and the provided use are in different blocks (it does not check this condition).<br class=""><br class="">================<br class="">Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2458<br class="">@@ +2457,3 @@<br class="">+ const BasicBlock *BB = SI->getParent();<br class="">+ if (!BB)<br class="">+ return false;<br class="">----------------<br class="">Can this really be called on un-inserted instructions?<br class=""><br class="">================<br class="">Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2461<br class="">@@ +2460,3 @@<br class="">+ auto *BI = dyn_cast_or_null<BranchInst>(BB->getTerminator());<br class="">+ if (!BI || BI->getNumSuccessors() != 2)<br class="">+ return false;<br class="">----------------<br class="">Or on instructions in incomplete blocks?<br class=""><br class=""><br class="">================<br class="">Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2975<br class="">@@ +2974,3 @@<br class="">+ ReplaceWithOpd = 1;<br class="">+ if (ConstantInt *CI = dyn_cast_or_null<ConstantInt>(Op1))<br class="">+ // When the constant operand of the select and cmp<br class="">----------------<br class="">Make this an else if?<br class=""><br class="">================<br class="">Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2988<br class="">@@ +2987,3 @@<br class="">+ ReplaceWithOpd = 2;<br class="">+ if (ReplaceWithOpd && I.isEquality()) {<br class="">+ // Replace select with operand on else path for EQ compares.<br class="">----------------<br class="">As we only actually perform the transform if I.isEquality(), can you host that part of the check up to be with the isChainSelectCmpBranch(SI) check?<br class=""><br class="">================<br class="">Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:100<br class="">@@ -98,1 +99,3 @@<br class=""> AU.addRequired<TargetLibraryInfo>();<br class="">+ AU.addRequired<DominatorTreeWrapperPass>();<br class="">+ AU.addPreserved<DominatorTreeWrapperPass>();<br class="">----------------<br class="">Are you going to separate this into a follow-up patch, or are you planning to provide compile-time measurements to before committing this one?<br class=""><br class=""><a href="http://reviews.llvm.org/D5258" class="">http://reviews.llvm.org/D5258</a><br class=""><br class=""><br class=""></div></blockquote></div><br class=""></div></body></html>