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

[Gerolf Hoflehner via llvm-commits]

That is still the same test case and the outcome should be independent of compiler revisions.  W/o reassoc the fill after the last call is not removed, but you get more ILP out of the 3 vadds. W reassoc there is no fill, but less ILP. You can’t get both higher ILP *and* a correct program, since in your proposed faster sequence xmm1 would not be initialized:


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

What value do you expect in xmm1 in the first vaddsd?

-Gerolf

> On Oct 21, 2015, at 8:07 AM, Sanjay Patel <spatel at rotateright.com> wrote:
> 
> spatel added a comment.
> 
> In http://reviews.llvm.org/D13417#271857, @Gerolf wrote:
> 
>> 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:
> 
> 
> If we have differing codegen/behavior, that would certainly explain why we're not reaching the same conclusion.
> 
> Let's make sure that we are using identical tests: I have confirmed on 2 machines with pristine checkouts of trunk (r250899) that I'm seeing the bad reassociation transform. The reference regression test case that I am referring to is here:
> test/CodeGen/X86/machine-combiner.ll, line 650:
> define double @already_reassociated() {
> 
> And this is the invocation for the checked output:
> ; RUN: llc -mtriple=x86_64-unknown-unknown -mcpu=x86-64 -mattr=avx -enable-unsafe-fp-math < %s | FileCheck %s --check-prefix=AVX
> 
> And these are the critical check lines:
> ; AVX-NEXT:  vaddsd  (%rsp), %xmm0, %xmm0
> ; AVX-NEXT:  vaddsd  8(%rsp), %xmm0, %xmm0
> ; AVX-NEXT:  vaddsd  16(%rsp), %xmm0, %xmm0
> 
> Does this reproduce in your environment?
> 
> 
> http://reviews.llvm.org/D13417
> 
> 
>