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

[Sanjay Patel via llvm-commits]

spatel added a comment.

In http://reviews.llvm.org/D11393#369670, @chandlerc wrote:

> I'm happy to say that the successor/predecessor stuff should be correct. The last pass I'm aware of calling these is MachineBlockPlacement which has pretty clear reliance on the CFG being accurate.
>
> If you want to double check, I actually think Quentin or Matze probably have the most context on the machine CFG at this point (more than I do honestly).
>
> I think the only thing I'd really suggest here is to really tighten up the testing. The code and logic looks really fantastic.


I was working my way back up through the tests and was going to suggest similar. Given that the patch has 2.5 LGTMs, I have nothing more to add. :)


================
Comment at: test/CodeGen/X86/fp-une-cmp.ll:79-83
@@ -79,3 +78,7 @@
 
-!1 = !{!"branch_weights", i32 1, i32 1000}
+; Test if the negation of the non-equality check between floating points are
+; translated to jnp followed by jne.
 
+; CHECK: jne
+; CHECK-NEXT: jnp
+define void @foo(float %f) {
----------------
chandlerc wrote:
> These checks don't really make sense to me. Why are they above the function, but the function has a CHECK-LABEL adn seemingly similar checks?
Those checks are the old lines from before the update check script was run. They should be deleted.

================
Comment at: test/CodeGen/X86/x86-analyze-branch-jne-jp.s:1-22
@@ +1,22 @@
+	.text
+	.file	"../llvm/test/CodeGen/X86/x86-analyze-branch-jne-jp.ll"
+	.globl	foo
+	.p2align	4, 0x90
+	.type	foo, at function
+foo:                                    # @foo
+	.cfi_startproc
+# BB#0:                                 # %entry
+	xorps	%xmm1, %xmm1
+	ucomiss	%xmm1, %xmm0
+	jne	.LBB0_2
+	jnp	.LBB0_1
+.LBB0_2:                                # %if.then
+	jmp	a                       # TAILCALL
+.LBB0_1:                                # %if.end
+	retq
+.Lfunc_end0:
+	.size	foo, .Lfunc_end0-foo
+	.cfi_endproc
+
+
+	.section	".note.GNU-stack","", at progbits
----------------
This file doesn't belong in the patch; it's a leftover from the earlier revision.


http://reviews.llvm.org/D11393