[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:
> Sorry, I got confused.
> X86InstrInfo::AnalyzeBranchImpl() also returns a vector of MachineInstructions, but the analyzeBranch() interface doesn't expose that, only the MachineOperands. How inconvenient.
> Anything we can do about this, or do you think it would be better not to touch this?
We could change analyzeBranch() I suppose, but it would probably break some out-of-tree backends, and I'm not sure it's worth it.

================
Comment at: lib/Target/X86/X86MCInstLower.cpp:509
@@ +508,3 @@
+    switch (MI->getOperand(1).getImm()) {
+    case X86::COND_O:  Opcode = X86::JO_1;  goto SetTailJmpOpcode;
+    case X86::COND_NO: Opcode = X86::JNO_1; goto SetTailJmpOpcode;
----------------
mkuper wrote:
> Can you use X86::GetCondBranchFromCond()?
Ah yes, much nicer.

================
Comment at: test/CodeGen/X86/conditional-tailcall.ll:21
@@ +20,3 @@
+; CHECK: jne bar
+; CHECK: encoding: [0x75,A]
+
----------------
(Forgot to reply to this the first time.)

When I worked on the patch, I initially forgot to change X86MCInstLower::Lower, which meant the printed assembly looked correct, but the binary instruction wasn't correct, so I wanted to test that.

I'll add a comment to the test.


https://reviews.llvm.org/D24108