[PATCH] D57881: [DTU] Refine the document of mutation APIs [NFC] (PR40528)

Jakub Kuderski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 13 08:45:25 PST 2019


kuhar accepted this revision.
kuhar added a comment.
This revision is now accepted and ready to land.

Looks better now, found just a couple of nits.



================
Comment at: llvm/include/llvm/Analysis/DomTreeUpdater.h:88
+  ///
+  /// These methods provide APIs for submitting updates to the DominatorTree and
+  /// the PostDominatorTree.
----------------
nit: s/APIs/API


================
Comment at: llvm/include/llvm/Analysis/DomTreeUpdater.h:91
+  ///
+  /// Note: There are two strategies to update the DominatorTree:
+  /// 1. Eager UpdateStrategy: Updates are submitted and then flushed
----------------
DominatorTree and PostDominatorTree?


================
Comment at: llvm/include/llvm/Analysis/DomTreeUpdater.h:95
+  /// 2. Lazy UpdateStrategy: Updates are submitted but only flushed when you
+  /// explicitly call flush APIs. It is recommended to use this update strategy
+  /// when you submit a bunch of updates multiple times which can then
----------------
s/call flush APIS/call the flush function


================
Comment at: llvm/include/llvm/Analysis/DomTreeUpdater.h:102
+  ///
+  /// Though GenericDomTree provides several update primitives,
+  /// it is not encouraged to use these APIs directly.
----------------
s/Though/Although


================
Comment at: llvm/include/llvm/Analysis/DomTreeUpdater.h:122
+  /// 2. It is illegal to submit any update that has already been submitted,
+  /// i.e. you are supposed not to insert an existent edge or delete a
+  /// nonexistent edge.
----------------
s/i.e./i.e.,


================
Comment at: llvm/include/llvm/Analysis/DomTreeUpdater.h:138
+  /// already been applied. }
   void insertEdge(BasicBlock *From, BasicBlock *To);
 
----------------
Please mark the method with: `LLVM_ATTRIBUTE_DEPRECATED`
This can happen in a separate patch.


================
Comment at: llvm/include/llvm/Analysis/DomTreeUpdater.h:148
+  /// submitted. }
   void insertEdgeRelaxed(BasicBlock *From, BasicBlock *To);
 
----------------
I thought that we still need the relaxed version in order to make JumpThreading code happy? Should it be deprecated?


================
Comment at: llvm/include/llvm/Analysis/DomTreeUpdater.h:157
+  /// submitted. }
   void deleteEdge(BasicBlock *From, BasicBlock *To);
 
----------------
Use `LLVM_ATTRIBUTE_DEPRECATED`


================
Comment at: llvm/include/llvm/Analysis/DomTreeUpdater.h:167
+  /// submitted. }
   void deleteEdgeRelaxed(BasicBlock *From, BasicBlock *To);
 
----------------
like above


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57881





More information about the llvm-commits mailing list