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

[Cong Hou via llvm-commits]

congh added inline comments.

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:4055
@@ +4054,3 @@
+      // is named COND_P_AND_NE.
+      BranchCode = X86::COND_P_AND_NE;
+    } else if ((OldBranchCode == X86::COND_NP && BranchCode == X86::COND_NE) ||
----------------
davidxl wrote:
> congh wrote:
> > davidxl wrote:
> > > congh wrote:
> > > > davidxl wrote:
> > > > > For B1, the condition is NP_OR_E, so why not using COND_NP_OR_E as the branch code? Is the new code needed? (after swapping TBB and FBB)
> > > > This is actually why this patch is created: COND_NP_OR_E is equivalent to COND_P_AND_NE, but the latter has the second condition reversed based on the former. COND_NP_OR_E has a JNP and JE, while COND_P_AND_NE has a JNP and JNE, or JE and JP. COND_NP_OR_E always has two identical targets, but COND_P_AND_NE doesn't. So they are different patterns. We use different names so that in X86::GetOppositeBranchCondition() we can get the reverse condition code for each other.
> > > I am not sure if we need to introduce the Opposite branch code for E_OR_NP.
> > > 
> > > Example:
> > > 
> > >   JE B1
> > >   JP B2
> > > B1 (fall through):
> > > 
> > > B2:
> > > 
> > > For this case, the Analyze branch can return success with the following tuple:
> > > {BranchCode =  E_OR_NP, TBB = B1, FBB = B2}
> > > 
> > > Later when insertBranch is called: there are two scenarios:
> > > 1) same as above where B1 is the fall through. The inserted code will be the same as above
> > > 2) B2 is the layout successor.  In this second case, the generated code sequence should look like:
> > >    JNP B1
> > >    JE   B1
> > > B2 (fall through):
> > > ..
> > > B1:
> > > 
> > > or 
> > >   JE B1
> > >   JNP B1
> > > B2:
> > >   ..
> > > B1:
> > > 
> > > Does it make sense?
> > In some places GetOppositeBranchCondition() is used to get the opposite branch code in order to generate an opposite branch. If we don't have an opposite branch code for COND_E_OR_NP, then how to reverse it?
> GetOppositteBranchCondition for some reason is never called with COND_E_OR_NP before, do you know why this path was not triggered ?
There are two places to reverse branches:

1. AnalyzeBranch(), in which GetOppositteBranchCondition() is called when there is a unconditional jump in the end of a MBB. If this is the case of a je+jnp branch, then a je+jp branch will be generated only based on jnp (GetOppositteBranchCondition returns jnp for a jp), as at this moment the COND_E_OR_NP pattern hasn't been recognized. If there is no jmp in the end,  GetOppositteBranchCondition() won't be called. A je+jp pattern is handled similarly. When there is no jmp following je+jnp, a COND_E_OR_NP pattern will be recognized.

2. Block placement: in this pass COND_E_OR_NP  won't be handled with a check.


http://reviews.llvm.org/D11393