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

[Sanjay Patel via llvm-commits]

spatel added a comment.

In http://reviews.llvm.org/D13417#261760, @stoklund wrote:

> In http://reviews.llvm.org/D13417#261721, @spatel wrote:
>
> > I think that shows the intended usage: slack is limited both by a dynamic calculation and a heuristic. Without those limits, we're almost certain to increase the critical path and lose performance (as shown in the examples here).
>
>
> The slack is simply computed relative to the critical path such that depth+slack+height = criticalpath.
>
> However, figuring out latencies of values returned by different function calls mostly amounts to guessing. Without any insight into the called function, there is not much to infer.


Agreed. Now that I understand MTM a bit more, I think the calls in this example could be replaced by other instructions, and we could show the same problem.

I think the key insight is to see that:

  critical path = max(depth, height)

But for the purposes of a *reassociation* transform, we are only concerned with reducing the depth, so we should not consider height (and therefore slack) in our cost equation for deciding if a reassociation transform is useful. This won't be true in general.


http://reviews.llvm.org/D13417