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

[Sanjay Patel via llvm-commits]

spatel added a comment.

[Gerolf's reply to my previous comment didn't show up here in Phab, so I'm copying it here for the sake of continuity and future reference.]

> It would be great to have an example that actually shows a better assembly sequence. Specifically I would like to see a similar example when there is no call in the block.


Certainly - it takes a little creativity to make this happen because x86 isel is aggressive (perhaps too aggressive) about folding loads into logic/math ops and our current reassociation does not consider instructions with memory ops as candidates for reassociation.

So we need to make sure the example has associative math ops with register operands and then create a trace with a critical path that is extended by heights (resource constraints) rather than limited by depths (data dependencies). We can do that by including a bunch of ops that use a different functional unit from our reassociable ops. Here's an example:

  define double @already_reassociated(double %a0, double %a1, double %a2, double %a3, i32* %b) {
    %b0 = getelementptr i32, i32* %b, i64 0
    %b1 = getelementptr i32, i32* %b, i64 2
    %b2 = getelementptr i32, i32* %b, i64 4
    %b3 = getelementptr i32, i32* %b, i64 6
  
    %i0 = load i32, i32* %b0
    %i1 = load i32, i32* %b1
    %i2 = load i32, i32* %b2
    %i3 = load i32, i32* %b3
  
    %x0 = add i32 %i0, %i1  ; The integer ops are here simply to lengthen the critical path and expose the bug.
    %x1 = add i32 %i2, %i3
    %x2 = add i32 %x0, %x1
  
    %x3 = add i32 %x0, %x1
    %x4 = add i32 %x2, %i3
    %x5 = add i32 %x3, %x4
  
    %x6 = add i32 %x3, %x4
    %x7 = add i32 %x5, %x6
    %x8 = add i32 %x6, %x7
    store i32 %x8, i32* %b3
  
    %t0 = fadd double %a0, %a1    ; This is the same as before: these fadds should NOT be reassociated.
    %t1 = fadd double %a2, %a3
    %t2 = fadd double %t0, %t1
    ret double %t2
  }


http://reviews.llvm.org/D13417