[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#270790, @Gerolf wrote:

> 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).


I'm struggling to explain this any better than what is shown in the "already_reassociated()" test case included in this patch. Please let me know if there's some other format or some debug output that I can include for reference.

With current (broken) reassociation, the expected sequence for  (a + b) + (c + d) is being changed from:

  vaddsd  16(%rsp), %xmm1, %xmm1     <--- c + d
  vaddsd  (%rsp), %xmm0, %xmm0       <--- a + b              [independent]
  vaddsd  %xmm0, %xmm1, %xmm0        <--- (a + b) + (c + d)

To:

  vaddsd (%rsp), %xmm0, %xmm0        <--- a + b
  vaddsd 8(%rsp), %xmm0, %xmm0       <--- (a + b) + c        [dependent]
  vaddsd 16(%rsp), %xmm0, %xmm0      <--- ((a + b) + c) + d  [dependent]


http://reviews.llvm.org/D13417