[PATCH] D11660: [x86] reassociate integer multiplies using machine combiner pass

[Sanjay Patel]

spatel added inline comments.

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:6309
@@ -6307,1 +6308,3 @@
 
+  // For binary instructions that have a third source operand (integer ops have
+  // EFLAGS), that operand must be dead.
----------------
Gerolf wrote:
> The intention of the code is checking for a binary operator with EFLAG. When the flag is dead the function returns false.
> I see two things to think about: first, why not having a function that makes it explicit incl. an assertion that the third operand actually is an eflag? In the current implementation the assumptions are implicit, so a function would make them more explicit/clear.
> Second, the host function checks whether or not a virtual register is defined in \param Inst. It returns true when that is the case. Why doesn't the code check for virtual defs when eflags is dead? So I think its purpose is to eliminate dead instructions in general and binary operators with eflags is perhaps just a special case. 
Thanks, Gerolf. I agree that I should make this more clear by splitting off the check and adding an assert.

To make my intention for this check more clear: if the EFLAGS operand on an integer math op is *not* dead, then I was thinking that those ops are involved in some kind of overflow or other exception-checking sequence, so it's not safe to rearrange the math. If the EFLAGS operands *are* dead, then the ops are part of normal integer math, so it is safe to do the reassociation.


http://reviews.llvm.org/D11660