[PATCH] D136396: [X86] Enable reassociation for ADD instructions

[Guozhi Wei via Phabricator via llvm-commits]

Carrot added inline comments.


================
Comment at: llvm/test/CodeGen/X86/reassociate-add.ll:4
+
+; This file checks the reassociation of ADD instruction.
+; The two ADD instructions add v0,v1,t2 together. t2 has a long dependence
----------------
spatel wrote:
> RKSimon wrote:
> > pengfei wrote:
> > > No idea if we intended to not do reassociation in ADD instructions. 
> > > Is there problem when unexpected overflow/underflow may be generated during reassociation?
> > Not that I can think of - EFLAGS will be the same and we already handles the $dst=$src0 constraint
> I was working on this a long time ago (~2015), so it's hard to remember exactly, but I don't think there was a fundamental reason to exclude integer ADD. 
> 
> It just seemed like it did not have much potential gain with the limited register set and could interfere with other transforms like LEA formation. 
> 
> If there's evidence that this improves something (and doesn't cause regressions), then it should be ok.
EFLAGS may be different. Suppose we are adding three bytes, 250 + 10 + 10. 

  - If we add them in the order (250 + 10) + 10, the first ADD generates carry/overflow flags, the second ADD doesn't generate carry/overflow.

  - If we add them in the order (10 + 10) + 250, the first ADD doesn't generates carry/overflow flags, the second ADD generates these flags.

So we need to check if the definition of EFLAGS is dead.

Maybe this is the reason @spatel didn't add ADD instructions in 2015.

Thank @pengfei for the reminds.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136396/new/

https://reviews.llvm.org/D136396