<div dir="ltr"><div>Here's the -debug-pass=Structure output from an O2 build of the repro code I posted earlier.  The '+' lines are new with the change:</div><div><br></div><div><div> Pass Arguments:  -datalayout -notti -basictti -x86tti -no-aa -tbaa -scoped-noalias -assumption-tracker -targetlibinfo -basicaa -verify -simplifycfg -domtree -sroa -early-cse -lower-expect</div><div> Data Layout</div><div> No target information</div><div> Target independent code generator's TTI</div><div> X86 Target Transform Info</div><div> No Alias Analysis (always returns 'may' alias)</div><div> Type-Based Alias Analysis</div><div> Scoped NoAlias Alias Analysis</div><div> Assumption Tracker</div><div> Target Library Information</div><div> Basic Alias Analysis (stateless AA impl)</div><div>   FunctionPass Manager</div><div>     Module Verifier</div><div>     Simplify the CFG</div><div>     Dominator Tree Construction</div><div>     SROA</div><div>     Early CSE</div><div>     Lower 'expect' Intrinsics</div><div>-Pass Arguments:  -targetlibinfo -datalayout -notti -basictti -x86tti -no-aa -tbaa -scoped-noalias -assumption-tracker -basicaa -verify-di -ipsccp -globalopt -deadargelim -instcombine -simplifycfg -basiccg -prune-eh -inline-cost -inline -functionattrs -sroa -domtree -early-cse -lazy-value-info -jump-threading -correlated-propagation -simplifycfg -instcombine -tailcallelim -simplifycfg -reassociate -domtree -loops -loop-simplify -lcssa -loop-rotate -licm -loop-unswitch -instcombine -scalar-evolution -lcssa -indvars -loop-idiom -loop-deletion -function_tti -loop-unroll -memdep -mldst-motion -domtree -memdep -gvn -memdep -memcpyopt -sccp -instcombine -lazy-value-info -jump-threading -correlated-propagation -domtree -memdep -dse -adce -simplifycfg -instcombine -barrier -domtree -loops -loop-simplify -lcssa -branch-prob -block-freq -scalar-evolution -loop-vectorize -instcombine -scalar-evolution -slp-vectorizer -simplifycfg -instcombine -domtree -loops -loop-simplify -lcssa -scalar-evolution -function_tti -loop-unroll -alignment-from-assumptions -strip-dead-prototypes -globaldce -constmerge -verify -verify-di -print-module</div><div>+Pass Arguments:  -targetlibinfo -datalayout -notti -basictti -x86tti -no-aa -tbaa -scoped-noalias -assumption-tracker -basicaa -verify-di -ipsccp -globalopt -deadargelim -instcombine -simplifycfg -basiccg -prune-eh -inline-cost -inline -functionattrs -sroa -domtree -early-cse -lazy-value-info -jump-threading -correlated-propagation -simplifycfg -instcombine -tailcallelim -simplifycfg -reassociate -domtree -loops -loop-simplify -lcssa -loop-rotate -licm -loop-unswitch -instcombine -domtree -loops -scalar-evolution -loop-simplify -lcssa -indvars -loop-idiom -loop-deletion -function_tti -loop-unroll -memdep -mldst-motion -domtree -memdep -gvn -memdep -memcpyopt -sccp -instcombine -lazy-value-info -jump-threading -correlated-propagation -domtree -memdep -dse -adce -simplifycfg -instcombine -barrier -domtree -loops -loop-simplify -lcssa -branch-prob -block-freq -scalar-evolution -loop-vectorize -instcombine -domtree -loops -scalar-evolution -slp-vectorizer -simplifycfg -instcombine -domtree -loops -loop-simplify -lcssa -scalar-evolution -function_tti -loop-unroll -alignment-from-assumptions -strip-dead-prototypes -globaldce -constmerge -verify -verify-di -print-module</div><div> Target Library Information</div><div> Data Layout</div><div> No target information</div><div> Target independent code generator's TTI</div><div> X86 Target Transform Info</div><div> No Alias Analysis (always returns 'may' alias)</div><div> Type-Based Alias Analysis</div><div> Scoped NoAlias Alias Analysis</div><div> Assumption Tracker</div><div> Basic Alias Analysis (stateless AA impl)</div><div>   ModulePass Manager</div><div>     Debug Info Verifier</div><div>     Interprocedural Sparse Conditional Constant Propagation</div><div>     Global Variable Optimizer</div><div>     Dead Argument Elimination</div><div>     FunctionPass Manager</div><div>       Combine redundant instructions</div><div>       Simplify the CFG</div><div>     CallGraph Construction</div><div>     Call Graph SCC Pass Manager</div><div>       Remove unused exception handling info</div><div>       Inline Cost Analysis</div><div>       Function Integration/Inlining</div><div>       Deduce function attributes</div><div>       FunctionPass Manager</div><div>         SROA</div><div>         Dominator Tree Construction</div><div>         Early CSE</div><div>         Lazy Value Information Analysis</div><div>         Jump Threading</div><div>         Value Propagation</div><div>         Simplify the CFG</div><div>         Combine redundant instructions</div><div>         Tail Call Elimination</div><div>         Simplify the CFG</div><div>         Reassociate expressions</div><div>         Dominator Tree Construction</div><div>         Natural Loop Information</div><div>         Canonicalize natural loops</div><div>         Loop-Closed SSA Form Pass</div><div>         Loop Pass Manager</div><div>           Rotate Loops</div><div>           Loop Invariant Code Motion</div><div>           Unswitch loops</div><div>         Combine redundant instructions</div><div>+        Dominator Tree Construction</div><div>+        Natural Loop Information</div><div>         Scalar Evolution Analysis</div><div>+        Canonicalize natural loops</div><div>         Loop-Closed SSA Form Pass</div><div>         Loop Pass Manager</div><div>           Induction Variable Simplification</div><div>           Recognize loop idioms</div><div>           Delete dead loops</div><div>         Function TargetTransformInfo</div><div>         Loop Pass Manager</div><div>           Unroll loops</div><div>         Memory Dependence Analysis</div><div>         MergedLoadStoreMotion</div><div>         Dominator Tree Construction</div><div>         Memory Dependence Analysis</div><div>         Global Value Numbering</div><div>         Memory Dependence Analysis</div><div>         MemCpy Optimization</div><div>         Sparse Conditional Constant Propagation</div><div>         Combine redundant instructions</div><div>         Lazy Value Information Analysis</div><div>         Jump Threading</div><div>         Value Propagation</div><div>         Dominator Tree Construction</div><div>         Memory Dependence Analysis</div><div>         Dead Store Elimination</div><div>         Aggressive Dead Code Elimination</div><div>         Simplify the CFG</div><div>         Combine redundant instructions</div><div>     A No-Op Barrier Pass</div><div>     FunctionPass Manager</div><div>       Dominator Tree Construction</div><div>       Natural Loop Information</div><div>       Canonicalize natural loops</div><div>       Loop-Closed SSA Form Pass</div><div>       Branch Probability Analysis</div><div>       Block Frequency Analysis</div><div>       Scalar Evolution Analysis</div><div>       Loop Vectorization</div><div>       Combine redundant instructions</div><div>+      Dominator Tree Construction</div><div>+      Natural Loop Information</div><div>       Scalar Evolution Analysis</div><div>       SLP Vectorizer</div><div>       Simplify the CFG</div><div>       Combine redundant instructions</div><div>       Dominator Tree Construction</div><div>       Natural Loop Information</div><div>       Canonicalize natural loops</div><div>       Loop-Closed SSA Form Pass</div><div>       Scalar Evolution Analysis</div><div>       Function TargetTransformInfo</div><div>       Loop Pass Manager</div><div>         Unroll loops</div><div>       Alignment from assumptions</div><div>     Strip Unused Function Prototypes</div><div>     Dead Global Elimination</div><div>     Merge Duplicate Global Constants</div><div>     FunctionPass Manager</div><div>       Module Verifier</div><div>     Debug Info Verifier</div><div>     Print module to stderr</div></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 3, 2014 at 1:14 PM, Andrew Trick <span dir="ltr"><<a href="mailto:atrick@apple.com" target="_blank">atrick@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div class="h5"><blockquote type="cite"><div>On Nov 3, 2014, at 11:48 AM, Mark Heffernan <<a href="mailto:meheff@google.com" target="_blank">meheff@google.com</a>> wrote:</div><br><div><div dir="ltr">Hi Andrew,<div><br></div><div>(Let me know if someone else is more appropriate to review this).  This patch removes setPreservesCFG() from the instcombine pass.  The reason is that InstCombine can modify terminator instructions (branches) specifically the predicate.  Instcombine clears out dead blocks and replaces all the dead instruction uses with undef:</div><div><br></div><div><a href="http://llvm.org/docs/doxygen/html/InstructionCombining_8cpp_source.html#l02792" target="_blank">http://llvm.org/docs/doxygen/html/InstructionCombining_8cpp_source.html#l02792</a><br></div><div><br></div><div>These new undefs can be predicates of branches, and from the comment of setPreservesCFG():</div><div><div><br></div><div>// setPreservesCFG - This function should be called to by the pass, iff they do</div><div>// not:</div><div>//</div><div>//  1. Add or remove basic blocks from the function</div><div>//  2. Modify terminator instructions in any way.</div></div><div><br></div><div>Clearly instcombine is modifying terminator instructions so should not be calling setPreservesCFG().  The specific issue I ran into was a dead loop not being deleted because of stale analysis data.  Repro:</div><div><br></div><div><div>void foo(int *a, int start_x, int end_x, int start_y, int end_y, int k) {</div><div>  for (int y = start_y; y < end_y; y++) {</div><div>    for (int x = start_x; x < end_x; x++) {</div><div>      for (int i = 0; i < 1000; i++) {</div><div>        a[x + y + k*i] = 0;</div><div>      }</div><div>    }</div><div>  }</div><div>}</div></div><div><br></div><div># Dead loop left after loop deletion:</div><div>clang -S -emit-llvm foo.c -o - | opt -O1 -loop-rotate -loop-unswitch -instcombine -loop-deletion -S > bad.ll<br></div><div><br></div><div># Dead loop removed by splitting out loop-deletion into separate opt invocation:</div><div>clang -S -emit-llvm foo.c -o - | opt -O1 -loop-rotate -loop-unswitch -instcombine | opt -loop-deletion -S > good.ll<br></div><div><br></div><div>This patch fixes this discrepancy.</div></div></div></blockquote><div><br></div></div></div>This seems totally reasonable given the current (lack of) invalidation mechanisms.</div><div><br></div><div>For the record, can you report the difference between -debug-pass=Structure before and after output?</div><div><br></div><div>-Andy</div><div><br><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>Mark</div></div>
<span><no_cfg_preserve_instcombine.patch></span></div></blockquote></div><br></div></blockquote></div><br></div>