[PATCH] D21774: [X86] Transform setcc + movzbl into xorl + setcc

[Michael Kuperstein via llvm-commits]

mkuper added a comment.

Thanks Elena, Ahmed!

In http://reviews.llvm.org/D21774#468880, @ab wrote:

> Have you considered doing this in DAGToDAG?  I'm not sure you'll be able to model the imp-use of EFLAGS (IIRC that's also a longstanding deficiency of pattern matching: we can't turn explicit into implicit), but seems worth a try?


Yes, that's what I started out with. That really would have been simpler, but I couldn't find a way to make it work.


================
Comment at: lib/Target/X86/CMakeLists.txt:20
@@ -19,1 +19,3 @@
   X86FastISel.cpp
+  X86FixupBWInsts.cpp
+  X86FixupLEAs.cpp
----------------
ab wrote:
> Go ahead and reorder separately?
Will do.

================
Comment at: lib/Target/X86/X86FixupSetCC.cpp:119
@@ +118,3 @@
+        MachineInstr *FlagsDefMI = findFlagsImpDef(
+            MI.getParent(), MachineBasicBlock::reverse_iterator(&MI));
+        if (!FlagsDefMI)
----------------
delena wrote:
> When you go backward, you should ensure that MI.getOperand(0).getReg() is not used between setcc and FlagsDefMI, including the second one.
> 
> for example:
> 
>    test %eax, %eax
>    setcc %al
> 
> you can kill %eax before the "test".
> 
I'm not sure I understood the scenario. This is pre-RA, the instruction I'm adding writes into a new vreg, so it can't clobber any other register.
But it made me notice a different edge case. I'm not sure this happens in practice, but in theory, FlagsDefMI may also imp-use eflags, in which case the transformation is invalid. I'll update the patch.


http://reviews.llvm.org/D21774