[PATCH] D72837: [AggressiveInstCombine] Add support for select instructions

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D72837#1840886 <https://reviews.llvm.org/D72837#1840886>, @aymanmus wrote:

> IMHO, there is no added value to splitting to 2 patches, the select changed are too obvious, and the compare is strongly related to the select changes.


That's kind of the point: The select change is obvious and can land right away, the icmp part is trickier.

I'm not really familiar with this code, but coupling the icmp and the select handling seems like a bit of a hack: The transform is also possible if the select condition is unrelated to the trunc chain, and the transform is also possible if there is an icmp dependent on the trunc chain but not used in a select condition. I think the more principled approach would be to extend the code in https://github.com/llvm/llvm-project/blob/0e0c65264aeb6f66b6f711884c55cdbf66d975f6/llvm/lib/Transforms/AggressiveInstCombine/TruncInstCombine.cpp#L241-L256 to collect the extra use icmp.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72837/new/

https://reviews.llvm.org/D72837