[PATCH] D13417: [MachineCombiner] remove "slack" from critical path cost calculation (PR25016)

[Sanjay Patel via llvm-commits]

spatel added a comment.

In http://reviews.llvm.org/D13417#259830, @Gerolf wrote:

> Taking the slack into account was necessary for the madd combiner to avoid regressions on a number of benchmarks. The new instruction sequence may use up the slack w/o extending the critical path. This change makes the heuristic too conservative. We should not change an heuristic for a fast-math optimization that impacts everything else.


Thanks for the explanation, Gerolf. I'm still trying to come up with an example where the slack makes a positive difference. Do you have a test case that we can add to the regression suite?

The reassociation transform is not fast-math-only at this point. We're using it for integer logic (and/or/xor) and math (imul) with x86. I have delayed enabling it for integer add in the hope of shaking out bugs like this, but my hope is to allow that too eventually.


http://reviews.llvm.org/D13417