[PATCH] D41463: [CodeGen] Add a new pass to sink Copy instructions after RA

[Jun Bum Lim via Phabricator via llvm-commits]

junbuml added inline comments.


================
Comment at: lib/CodeGen/MachineCopySink.cpp:130
+  // Check if any register aliased with Reg is live-in in other successors.
+  for (auto *SI : CurBB.successors()) {
+    if (SI == BB)
----------------
mcrosier wrote:
> (A suggestion that can be ignored if you disagree, but) I think this might be easier to understand if this loop and the loop on line 118 were combine.
> 
> Something like:
>   MachineBasicBlock *BB = nullptr;
>   for (auto *SI : CurBB.successors()) {
>     // Try to find a single sinkable successor in which Reg is live-in.
>     if (SinkableBBs.count(SI) && SI->isLiveIn(Reg)) {
>       if (BB)
>         return nullptr;
>       BB = SI;
>       continue;
>     }
>     // Check if Reg or any register aliased with Reg is live-in in other successors.
>     if (SI->isLiveIn(Reg))
>       return nullptr;
> 
>     for (const auto LI : SI->liveins())
>       if (AliasedRegs.count(LI.PhysReg))
>         return nullptr;
>   }
> 
> Of course this means that you'll have to change SinkableBBs to a Set/SmallSet, which I believe you can iterate over..
I agree that merging these two loops is easier to read, but I intentionally kept these two loops separate because I wanted to take the second loop only when we found a single sinkable successor.


https://reviews.llvm.org/D41463