[PATCH][instcombine] Remove preservesCFG from instcombine

[Mark Heffernan]

On Tue, Nov 4, 2014 at 12:28 AM, Nick Lewycky <nicholas at mxc.ca> wrote:

> NAK. Please don't commit this.


I committed it yesterday with r221223.  The submitted code replaced
setPreservesCFG with addPreserved<DominatorTreeWrapperPass> and
addPreserved<LoopInfo>.

  The reason
>
>> is that InstCombine can modify terminator instructions (branches)
>> specifically the predicate.
>>
>
> This is the thing where it removes an '%newcond = xor %cond, true' and
> switches 'br %newcond, label %A, label %B' with 'br %cond, label %B, label
> %A'?
>
> If it's actually causing you problems, please just move that transform to
> simplifycfg instead of removing preserves-cfg from instcombine.


The problem I encountered is in instcombine where it clears out
instructions from unreachable BB's and replaces their uses with undef:

http://llvm.org/docs/doxygen/html/InstructionCombining_8cpp_source.html#l02796


These uses can include branch predicates.  Loop simplify then turns these
'bra undefs' at loop latches to always exit so loopsimplify is not
preserved so setPreservesCfg should not be used by instcombine.... or am I
missing something?

Mark

Nick
>
>
>   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
>>
>> 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.
>>
>> Mark
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141104/2fa4d787/attachment.html>