[PATCH] D54237: Constant folding and instcombine for saturating adds

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D54237#1291365, @bjope wrote:

> I'm no expert on how things are divided between InstructionSimplify and InstCombine, but shouldn't the simple folds be in InstructionSimplify?


The rule is that if you don't create any new variable values (ie, you are returning an existing value or constant), then it should go in InstSimplify. Note that InstSimplify is used as an analysis by other passes like GVN, so putting transforms in there improves things beyond InstCombine.

I see that this patch already has 2 approvals, so I won't block it, but we really should have the regression tests in the corresponding test directory (and using the appropriate pass in the RUN line). So the constant folding hunk of this patch should really be its own commit with tests in test/Analysis/ConstantFolding/, and the InstSimplify hunk of this patch should be its own commit with tests in test/Transforms/InstSimplify. Please make those changes as a follow-up if this is committed initially as a single patch.


https://reviews.llvm.org/D54237