[PATCH] D13417: [MachineCombiner] make slack optional in critical path cost calculation (PR25016)

[Gerolf Hoflehner via llvm-commits]

Sorry, this has been a while. Please send me the instruction sequence you get w/ and w/o reassociation. Even w/o reassoc I see three dependent fadds (although there are four rather than three).

In the case of mul+add -> madd the combine could increase depth but that is fine as long as it does not increase the critical path. The combiners estimate looks fine to me. As long as the estimate holds it should not hurt performance even when reassoc increases depth  - unless the slack is wrong/or very conservative. If that is the case and it can’t be fixed then I think we have to pursue your idea and add the capability of associating a local (per pattern) estimator.

-Gerolf

> On Oct 13, 2015, at 7:46 AM, Sanjay Patel <spatel at rotateright.com> wrote:
> 
> spatel added a comment.
> 
> Ping.
> 
> 
> http://reviews.llvm.org/D13417
> 
> 
>