[PATCH] D24108: X86: Fold tail calls into conditional branches where possible (PR26302)

[Hans Wennborg via llvm-commits]

hans added inline comments.

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:4079
@@ -4010,2 +4078,3 @@
+
 // Given a MBB and its TBB, find the FBB which was a fallthrough MBB (it may
 // not be a fallthrough MBB now due to layout changes). Return nullptr if the
----------------
mkuper wrote:
> Yeah, you're right, I can't think of a really nice way to handle this, since the analyzeBranches() call is in in a generic part of this patch, not x86-specific. And the search here should normally be really short anyway.
> 
> But could you please leave a comment documenting this, in case someone decides to refactor this later.
> 
Adding a comment.

================
Comment at: test/CodeGen/X86/conditional-tailcall.ll:22
@@ +21,3 @@
+; Check that the asm doesn't just look good, but uses the correct encoding.
+; CHECK: encoding: [0x75,A]
+
----------------
mkuper wrote:
> I'm a bit confused about this. Without the change to X86MCInstLower, I'd expect you to get complete nonsense, not a poorly encoded jmp.
> Anyway, that's a problem with my understanding of this, not your patch. :-)
Yes, the bits were garbage, but with the original version of my patch it still got printed as "jne" in the assembly. That wouldn't happen with the current version, but it still seems like a good idea to do a quick check of the encoding.


https://reviews.llvm.org/D24108