[PATCH] D59696: [CGP] Build the DominatorTree lazily

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson marked 4 inline comments as done.
tejohnson added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:300-301
 
+    /// Dominator tree is built lazily. This is reset when the
+    /// function is changed.
+    std::unique_ptr<DominatorTree> DT;
----------------
spatel wrote:
> We should add the reasoning for this choice in the code comment. 
> Something like:
> Creating a dominator may be expensive, so we only do this lazily and update when required.
Modified comment.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:390-395
+    bool optimizeCmp(CmpInst *Cmp, const TargetLowering &TLI,
+                     const DataLayout &DL, bool &ModifiedDT);
+    bool combineToUSubWithOverflow(CmpInst *Cmp, const TargetLowering &TLI,
+                                   const DataLayout &DL, bool &ModifiedDT);
+    bool combineToUAddWithOverflow(CmpInst *Cmp, const TargetLowering &TLI,
+                                   const DataLayout &DL, bool &ModifiedDT);
----------------
spatel wrote:
> I don't like the idea of passing a shadowed name version of the class member as a param (and we already found a bug related to that). Can we make these member functions as a preliminary NFC commit?
Good catch on the TLI and TL member variables! I fixed that part and committed the restructuring in r356857.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59696