llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.84k stars 11.47k forks source link

Extra moves due to failure to commute sub+add to sub+sub #33558

Open RKSimon opened 7 years ago

RKSimon commented 7 years ago
Bugzilla Link 34210
Version trunk
OS Windows NT
CC @LebedevRI,@rotateright,@ZviRackover

Extended Description

int foo(int x, int y, int *diff)
{
    *diff += y - x;
    return y;
}

define i32 @foo(i32, i32 returned, i32* nocapture) local_unnamed_addr #0 {
  %4 = sub i32 %1, %0
  %5 = load i32, i32* %2
  %6 = add nsw i32 %4, %5
  store i32 %6, i32* %2
  ret i32 %1
}

foo:
  movl %esi, %eax
  subl %edi, %eax
  addl %eax, (%rdx)
  movl %esi, %eax
  retq

But we could avoid the extra mov by performing:

foo:
  subl %esi, %edi
  subl %edi, (%rdx)
  movl %esi, %eax
  retq

Which would be equivalent to:

int bar(int x, int y, int *diff)
{
    *diff -= x - y;
    return y;
}

Which annoyingly gets canonicalized to something very similar:

define i32 @bar(i32, i32 returned, i32* nocapture) local_unnamed_addr #0 {
  %4 = load i32, i32* %2
  %5 = sub i32 %1, %0
  %6 = add i32 %5, %4
  store i32 %6, i32* %2
  ret i32 %1
}

bar:
  movl %esi, %eax
  subl %edi, %eax
  addl %eax, (%rdx)
  movl %esi, %eax
  retq

I'm assuming because add is more commutable.......

RKSimon commented 4 years ago

Current Codegen: https://godbolt.org/z/sHjPKH

Still not as good as it could be

llvmbot commented 7 years ago

I was expecting that it'd probably be done as part of commutation during TwoAddressInstructionPass/RegAlloc

Yes, good one, Simon. Delaying this until we hit the backend is a better choice.

RKSimon commented 7 years ago

I was expecting that it'd probably be done as part of commutation during TwoAddressInstructionPass/RegAlloc

llvmbot commented 7 years ago

Maybe a job for reassociate. I'm not sure this is always profitable in practice.