[PATCH] D28114: [StructurizeCfg] Update dominator info.

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: include/llvm/Support/GenericDomTree.h:581
+  /// \param DomBB CFG node that is dominator for BB, nullptr if the new CFG
+  ///              node becomes a new root.
+  /// \returns New dominator tree node that represents new CFG node.
----------------
you should add that the old root node is assumed to be a successor of the new root node. I'm somewhat concerned that this is not really workable for post-dom trees where multiple roots are possible. Thoughts?



================
Comment at: include/llvm/Support/GenericDomTree.h:587
     DFSInfoValid = false;
-    return (DomTreeNodes[BB] = IDomNode->addChild(
+    if (DomTreeNodeBase<NodeT> *IDomNode = getNode(DomBB)) {
+      return (DomTreeNodes[BB] = IDomNode->addChild(
----------------
You're calling getNode on a null pointer? That seems wasteful, because you're doing a map lookup  on a nullptr.


https://reviews.llvm.org/D28114