[PATCH] D46380: [InstSimplify] Adding safety checks for 'shl', 'ashr' and 'lshr'

[Omer Paparo Bivas via Phabricator via llvm-commits]

opaparo added a comment.

In https://reviews.llvm.org/D46380#1096108, @spatel wrote:

> 1. This is an instsimplify patch - the review title should be changed; there should be minimal tests for each transform under tests/Transforms/InstSimplify.


The title was changed. However, those changes do not affect InstSimplify directly, as they are not used by this pass. The only functional change is in InstCombine, which uses these parts of InstSimplify as a library.

In https://reviews.llvm.org/D46380#1096108, @spatel wrote:

> 2. This is using ValueTracking which can be expensive in compile-time - do you have any stats for how often this fires, perf improvements, compile-time regressions? (adding Eli for thoughts about where we would draw that line because I really don't know)


After a close look I discovered some compile-time regressions, as you predicted. I removed the safety checks for 'add', 'sub' and 'mul', which require ValueTracking (so now there are only safety checks for 'shl', 'ashr' and 'lshr'). That elimintated those regressions.


https://reviews.llvm.org/D46380