[PATCH] D49944: [Dominators] Make applyUpdate's documentation less confusing

Jakub Kuderski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 27 15:20:21 PDT 2018


kuhar created this revision.
kuhar added reviewers: chandlerc, asbirlea, NutshellySima.
Herald added a reviewer: grosser.

It was pointed out by @chandlerc that it's not clear whether both applyUpdates and insert/deleteEdge can be used to perform multiple updates.

IMO, the confusing part was that the comment above applyUpdates made a comparison of expected update time between calling it and calling insert/deleteEdge multiple times. It's generally not possible to safely call insert/deleteEdge multiple times, which documentation for each of the 3 functions warns about, so the whole comparison makes very little sense. On top of that, the comment is already lengthy, so I think it's best to just get rid of this comparison.


Repository:
  rL LLVM

https://reviews.llvm.org/D49944

Files:
  include/llvm/Support/GenericDomTree.h


Index: include/llvm/Support/GenericDomTree.h
===================================================================
--- include/llvm/Support/GenericDomTree.h
+++ include/llvm/Support/GenericDomTree.h
@@ -530,11 +530,10 @@
   /// CFG about its children and inverse children. This implies that deletions
   /// of CFG edges must not delete the CFG nodes before calling this function.
   ///
-  /// Batch updates should be generally faster when performing longer sequences
-  /// of updates than calling insertEdge/deleteEdge manually multiple times, as
-  /// it can reorder the updates and remove redundant ones internally.
-  /// The batch updater is also able to detect sequences of zero and exactly one
-  /// update -- it's optimized to do less work in these cases.
+  /// The applyUpdates function can reorder the updates and remove redundant
+  /// ones internally. The batch updater is also able to detect sequences of
+  /// zero and exactly one update -- it's optimized to do less work in these
+  /// cases.
   ///
   /// Note that for postdominators it automatically takes care of applying
   /// updates on reverse edges internally (so there's no need to swap the


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D49944.157786.patch
Type: text/x-patch
Size: 1172 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180727/0fef89d8/attachment.bin>


More information about the llvm-commits mailing list