<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Nov 3, 2014, at 11:48 AM, Mark Heffernan <<a href="mailto:meheff@google.com" class="">meheff@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Hi Andrew,<div class=""><br class=""></div><div class="">(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 class=""><br class=""></div><div class=""><a href="http://llvm.org/docs/doxygen/html/InstructionCombining_8cpp_source.html#l02792" class="">http://llvm.org/docs/doxygen/html/InstructionCombining_8cpp_source.html#l02792</a><br class=""></div><div class=""><br class=""></div><div class="">These new undefs can be predicates of branches, and from the comment of setPreservesCFG():</div><div class=""><div class=""><br class=""></div><div class="">// setPreservesCFG - This function should be called to by the pass, iff they do</div><div class="">// not:</div><div class="">//</div><div class="">// 1. Add or remove basic blocks from the function</div><div class="">// 2. Modify terminator instructions in any way.</div></div><div class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class=""><div class="">void foo(int *a, int start_x, int end_x, int start_y, int end_y, int k) {</div><div class=""> for (int y = start_y; y < end_y; y++) {</div><div class=""> for (int x = start_x; x < end_x; x++) {</div><div class=""> for (int i = 0; i < 1000; i++) {</div><div class=""> a[x + y + k*i] = 0;</div><div class=""> }</div><div class=""> }</div><div class=""> }</div><div class="">}</div></div><div class=""><br class=""></div><div class=""># Dead loop left after loop deletion:</div><div class="">clang -S -emit-llvm foo.c -o - | opt -O1 -loop-rotate -loop-unswitch -instcombine -loop-deletion -S > bad.ll<br class=""></div><div class=""><br class=""></div><div class=""># Dead loop removed by splitting out loop-deletion into separate opt invocation:</div><div class="">clang -S -emit-llvm foo.c -o - | opt -O1 -loop-rotate -loop-unswitch -instcombine | opt -loop-deletion -S > good.ll<br class=""></div><div class=""><br class=""></div><div class="">This patch fixes this discrepancy.</div></div></div></blockquote><div><br class=""></div>This seems totally reasonable given the current (lack of) invalidation mechanisms.</div><div><br class=""></div><div>For the record, can you report the difference between -debug-pass=Structure before and after output?</div><div><br class=""></div><div>-Andy</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">Mark</div></div>
<span id="cid:43D7B4F0-4A77-4F56-9DB7-7F0E8020308A@apple.com"><no_cfg_preserve_instcombine.patch></span></div></blockquote></div><br class=""></body></html>