[PATCH] D107534: Add a pass to garbage-collect empty basic blocks after code generation.

[Rahman Lavaee via Phabricator via llvm-commits]

rahmanl added a subscriber: dblaikie.
rahmanl added inline comments.


================
Comment at: llvm/lib/CodeGen/GCEmptyBasicBlocks.cpp:53
+    if (HasAnyRealCode)
+      continue;
+
----------------
wenlei wrote:
> tmsriram wrote:
> > If the block has no real instructions but debug instructions, is it correct to nuke it?
> For debuggbility, probably not. Relatedly the current implementation also keep the block if it contains pseudo-probe, which is what we want. 
I am guessing debug-info instructions in a basic block with zero binary size is never used. Any ideas @dblaikie ?

@wenlei My initial idea was to GC all empty blocks before inserting pseudo probes. Now I realize pseudo probes may be inserted anytime. But I still think we can remove empty blocks with pseudo probes at this point of CodeGen. WDYT?


================
Comment at: llvm/lib/CodeGen/GCEmptyBasicBlocks.cpp:58
+    // predecessors. Although this shouldn't happen in valid code.
+    if (NextMBB == E && !Preds.empty())
+      continue;
----------------
tmsriram wrote:
> If it is the last block it must have predecessors or you couldn't get to it, so !Preds.empty() is redundant.
I found one case of a function with a single empty basic block which prompted me to add this check.


================
Comment at: llvm/lib/CodeGen/GCEmptyBasicBlocks.cpp:70
+      if (JTI)
+        JTI->ReplaceMBBInJumpTables(&*MBB, &*NextMBB);
+    }
----------------
tmsriram wrote:
> Should we add the non-real instructions in this block to the beginning of the next block too?
I am not sure we can. For example, a register kill instruction cannot be safely moved into the new block, if that block has other predecessors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107534