[PATCH][instcombine] Remove preservesCFG from instcombine

[Andrew Trick]

> On Nov 3, 2014, at 11:48 AM, Mark Heffernan <meheff at google.com> wrote:
> 
> Hi Andrew,
> 
> (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:
> 
> http://llvm.org/docs/doxygen/html/InstructionCombining_8cpp_source.html#l02792 <http://llvm.org/docs/doxygen/html/InstructionCombining_8cpp_source.html#l02792>
> 
> These new undefs can be predicates of branches, and from the comment of setPreservesCFG():
> 
> // setPreservesCFG - This function should be called to by the pass, iff they do
> // not:
> //
> //  1. Add or remove basic blocks from the function
> //  2. Modify terminator instructions in any way.
> 
> 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:
> 
> void foo(int *a, int start_x, int end_x, int start_y, int end_y, int k) {
>   for (int y = start_y; y < end_y; y++) {
>     for (int x = start_x; x < end_x; x++) {
>       for (int i = 0; i < 1000; i++) {
>         a[x + y + k*i] = 0;
>       }
>     }
>   }
> }
> 
> # Dead loop left after loop deletion:
> clang -S -emit-llvm foo.c -o - | opt -O1 -loop-rotate -loop-unswitch -instcombine -loop-deletion -S > bad.ll
> 
> # Dead loop removed by splitting out loop-deletion into separate opt invocation:
> clang -S -emit-llvm foo.c -o - | opt -O1 -loop-rotate -loop-unswitch -instcombine | opt -loop-deletion -S > good.ll
> 
> This patch fixes this discrepancy.

This seems totally reasonable given the current (lack of) invalidation mechanisms.

For the record, can you report the difference between -debug-pass=Structure before and after output?

-Andy

> 
> Mark
> <no_cfg_preserve_instcombine.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141103/7ab39dab/attachment.html>