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

[Sanjay Patel via Phabricator via llvm-commits]

spatel 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;
----------------
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.


================
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);
----------------
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?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59696