[PATCH] D11393: [X86] Allow X86::COND_NE_OR_P and X86::COND_NP_OR_E to be reversed.

[Cong Hou]

congh added inline comments.

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:3530
@@ -3568,3 +3529,3 @@
     // we could handle more patterns here, but we shouldn't expect to see them
     // if instruction selection has done a reasonable job.
     if ((OldBranchCode == X86::COND_NP &&
----------------
Gerolf wrote:
> It would be nice if you added a FIXME even though this is not  part of your code. Any assumption about the IS patterns  should be made explicit and checked with  an assertion.
Thanks for your review! I have added assertion checking if two destinations are identical for X86::COND_NP_OR_E and X86::COND_NE_OR_P. For X86::COND_P_AND_NE and X86::COND_E_AND_NP, however, I am not sure we should assert that they have different destinations. This is because it is still OK even when they have the same destination.

And do still need a FIXME?

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:3541
@@ -3579,1 +3540,3 @@
       BranchCode = X86::COND_NE_OR_P;
+    else if ((OldBranchCode == X86::COND_NE &&
+              BranchCode == X86::COND_NP) ||
----------------
Gerolf wrote:
> Perhaps I'm only iterating what Duncan said. What I'm confused by is that the previous pattern are symmetrical: For example the first case is NP && E or E && NP while the new cases are asymmetrical like here NE && NP or P && E (as opposed to  NE && NP or NP && NE, which is what I would expect from the handling of the previous pattern). At least there need to be a good explanation (comment) for this.
This condition has two instructions with two different destinations, and the second destination is the true BB. Therefore if we have NE then NP, then the true body can only be reached with !NE && NP; that is E && NP. If we have P then E, the true body can only be reached with !P && E; that is NP && E. And then we got two equivalent conditions. I have added a comment explaining it.

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:3554
@@ -3582,1 +3553,3 @@
 
+    // Only handle the case where all conditional branches branch to the same
+    // destination except X86::COND_P_AND_NE and X86::COND_E_AND_NP, which have
----------------
Gerolf wrote:
> I would need a picture and examples to understand for which conditions chains the TBB condition is relevant. 
I have added a comment showing examples of X86::COND_P_AND_NE in which two branch destinations are different.


http://reviews.llvm.org/D11393