[PATCH] D22864: [InstCombine] Combine unfolding and optimization of casts in zext(or(icmp, icmp))

[Matthias J. Reisinger via llvm-commits]

mreisinger created this revision.
mreisinger added reviewers: grosser, spatel.
mreisinger added a subscriber: llvm-commits.

In commit r275989 we tried to enable the folding of `logic(cast(icmp), cast(icmp))` to `cast(logic(icmp, icmp))` which had to be reverted in r276106 because it could lead to an endless loop in InstCombine (also see http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160718/374347.html). The root of this problem is that in `visitZExt()` in InstCombineCasts.cpp there also exists a reverse of the above folding transformation, that unfolds `zext(or(icmp, icmp))` to `or(zext(icmp), zext(icmp))` in order to expose `zext(icmp)` operations which would then possibly be eliminated by subsequent iterations of InstCombine. However, before these `zext(icmp)` would be eliminated the folding from r275989 could kick in and cause InstCombine to endlessly switch back and forth between the folding and the unfolding transformation.

To prevent this we now combine the `zext`-unfolding and the elimination of the exposed `zext(icmp)` to happen at one go. This enables us to still allow the cast-folding in `logic(cast(icmp), cast(icmp))` without entering an endless loop again.

Details on the submitted changes:
- In `visitZExt()` we combine the unfolding and optimization of `zext` instructions. Note the `cast` operations on the `Value`s returned by `Builder->CreateZExt()` which we placed because (a) we assume that the `Builder` indeed creates a `ZExtInst` at this place and (b) `transformZExtICmp()` expects a `ZExtInst` to be passed.
- In `transformZExtICmp()` we have to use `Builder->CreateIntCast()` instead of `CastInst::CreateIntegerCast()` to make sure that the new `CastInst` is inserted in a `BasicBlock`. The new calls to `transformZExtICmp()` that we introduce in `visitZExt()` would otherwise cause according assertions to be triggered (in our case this happend, for example, with lnt for the MultiSource/Applications/sqlite3 and SingleSource/Regression/C++/EH/recursive-throw tests). The subsequent usage of `replaceInstUsesWith()` is necessary to ensure that the new `CastInst` replaces the `ZExtInst` accordingly.
- In InstCombineAndOrXor.cpp we again allow the folding of casts on `icmp` instructions.
- The instruction order in the optimized IR for the zext-or-icmp.ll test case is different with the introduced changes
- The test cases in `zext.ll` have been adopted from the reverted commits r275989 and r276105.

https://reviews.llvm.org/D22864

Files:
  lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
  lib/Transforms/InstCombine/InstCombineCasts.cpp
  test/Transforms/InstCombine/zext-or-icmp.ll
  test/Transforms/InstCombine/zext.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D22864.65722.patch
Type: text/x-patch
Size: 6067 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160727/bab3b47a/attachment.bin>