[PATCH] D106317: [SimplifyCFG] performBranchToCommonDestFolding(): form block-closed SSA form before cloning instructions (PR51125)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1108
+      if (auto *PN = dyn_cast<PHINode>(UI)) {
         SSAUpdate.RewriteUse(U);
+      } else {
----------------
nikic wrote:
> Do we still need SSAUpdater here? Can this be just `if (PN->getIncomingBlock(U) != BB) U.set(NewBonusInst);` or so?
Hmm, yes, this is better.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:3080
+  // and that makes miscompiles easy. Let's make it explicit instead,
+  // by forming sudo-LCSSA.
+  for (Instruction &BonusInst :
----------------
nikic wrote:
> This comment would really benefit from a description of what problem this is solving and how it does so. "makes miscompiles easy" doesn't really tell us anything and "sudo-LCSSA" form is somewhat confusing when not talking about loops.
Reworded, but not sure how better this is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106317