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

[Gerolf Hoflehner via llvm-commits]

Perhaps there is another change in your tree.  We are looking at different code and/or I don’t know how your reference sequence for your test case. With fast-math and reassociation turned off I’m getting:

        vmovsd  8(%rsp), %xmm1          # 8-byte Reload
                                        # xmm1 = mem[0],zero
        vaddsd  16(%rsp), %xmm1, %xmm1  # 8-byte Folded Reload
        vaddsd  (%rsp), %xmm0, %xmm0    # 8-byte Folded Reload
        vaddsd  %xmm0, %xmm1, %xmm0
Which is happens to be the same code I get w/o fast-math.

That should not be faster than fast-math with reassociation.

I don’t understand why the equation *holds* although you measure a performance loss. I very much prefer a single equation that holds for all pattern.

-Gerolf

PS:
reassoc.ll

declare double @bar()
define double @foo4_reassociated() {
    %a = call double @bar()
    %b = call double @bar()
    %c = call double @bar()
    %d = call double @bar()
    %t0 = fadd double %a, %b 
    %t1 = fadd double %c, %d 
    %t2 = fadd double %t0, %t1 
    ret double %t2
}

> On Oct 20, 2015, at 9:30 AM, Sanjay Patel <spatel at rotateright.com> wrote:
> 
> 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
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151020/bfbb1e93/attachment-0001.html>