[PATCH] D54411: [Codegen] Merge tail blocks with no successors after block placement

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 marked 2 inline comments as done.
xbolva00 added inline comments.


================
Comment at: lib/CodeGen/BranchFolding.cpp:1074
+  if (!EnableTailMerge)
+    return MadeChange;
 
----------------
Jim wrote:
> This change is it neccessary?
I think it is fine since we touch this code/function, we should clang-format it too..


================
Comment at: test/CodeGen/X86/tail-opts.ll:439
+  %1 = icmp eq i32 undef, 0
+  br i1 %1, label %return, label %bb12
 
----------------
Jim wrote:
> RKSimon wrote:
> > dmgreen wrote:
> > > Can you explain this change? The comment makes it look like it should be the same test as above.
> > > 
> > > But it somehow has different fall-through branches with the switches?
> > icmp with undef will now constant fold in SelectionDAG - you're probably being affected by that
> This testcase tests that two `tail_call_me` can't be merged into one for performance issue.
> But bby and bbx are likely the same. This patch makes bby and bbx merge into one. 
> And make `tail_call_me` only need one. 
> So the two comparison of switch in bby and bbx should be changed difference.
> 
> This is my testcase to keep `tail_call_me` tail-duplicated.
> 
> ```
> define void @two_nosize(i32 %x, i32 %y, i32 %z) nounwind {
> entry:
>   %0 = icmp eq i32 %x, 0
>   br i1 %0, label %bbx, label %bby
> 
> bby:
>   switch i32 %y, label %bb7 [
>     i32 0, label %return
>   ]
> 
> bb7:
>   store volatile i32 0, i32* @XYZ
>   tail call void @tail_call_me()
>   ret void
> 
> bbx:
>   switch i32 %z, label %bb12 [
>     i32 -1, label %return
>   ]
> 
> bb12:
>   store volatile i32 0, i32* @XYZ
>   tail call void @tail_call_me()
>   ret void
> 
> return:
>   ret void
> }
> 
> ```
> Let two comparison compare with 0 or -1. So that tail call `tail_call_me` can be put on the fall through edge.
> 
Thanks for testcase, I will update it.


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

https://reviews.llvm.org/D54411