[PATCH] D140931: Improve and enable folding of conditional branches with tail calls.

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:3030-3032
+  MIB.addImm(0);                           // Stack offset (not used).
+  MIB->addOperand(BranchCond[0]);          // Condition.
+  MIB.copyImplicitOps(TailCall);           // Regmask and (imp-used) parameters.
----------------
pengfei wrote:
> Revert the format change in this file.
> Revert the format change in this file.

Will do for v4.


================
Comment at: llvm/test/CodeGen/X86/segmented-stacks.ll:1725
-; X86-Linux-NEXT:  # %bb.2:
-; X86-Linux-NEXT:    jmp callee at PLT # TAILCALL
-; X86-Linux-NEXT:  .LBB8_1:
----------------
pengfei wrote:
> It doesn't look like multi predecessors.
> It doesn't look like multi predecessors.

Hmm?


================
Comment at: llvm/test/CodeGen/X86/segmented-stacks.ll:1729
 ; X86-Linux-NEXT:    retl
 ; X86-Linux-NEXT:    jmp callee at PLT # TAILCALL
 ;
----------------
pengfei wrote:
> This looks like dead code?
> This looks like dead code?

Appears to be. Not sure whats causing that. Is `__morestack` doing something tricky?


================
Comment at: llvm/test/CodeGen/X86/tail-opts.ll:866
 ; CHECK-NEXT:    cmpl $128, %edi
-; CHECK-NEXT:    jne .LBB14_8
-; CHECK-NEXT:  # %bb.5: # %return
+; CHECK-NEXT:    jne tail_call_me # TAILCALL
+; CHECK-NEXT:  # %bb.7: # %return
----------------
pengfei wrote:
> goldstein.w.n wrote:
> > pengfei wrote:
> > > This looks like increasing size some time. Is this caused by the change of checking for all predecessors?
> > > This looks like increasing size some time. Is this caused by the change of checking for all predecessors?
> > 
> > Is this increasing size? Seems to be one less basic-block to me and -1 instruction. I think the numbering just gets weird and it jumps from `bb.2` -> `bb.5`.
> `jne rel8` vs. `jne rel32`. 3 bytes longer and no instruction reduced.
> `jne rel8` vs. `jne rel32`. 3 bytes longer and no instruction reduced.

Yeah, you're right, its because the tail call `MBB` is used by multiple branches, some of which can be dropped, but not all.

We could make it so that for `-Os` removing the tail calls is all or nothing (on the assumption that the `MBB` is more likely to be imm8 encodable than the jmp, although the alternative is also possible).

What do you think?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140931/new/

https://reviews.llvm.org/D140931