[Dominators] Add isUpdateLazy() method to the DomTreeUpdater

[NutshellySima]

Summary

Previously, when people need to deal with DTU with different UpdateStrategy using different actions, they need to

if (DTU.getUpdateStrategy() == DomTreeUpdater::UpdateStrategy::Lazy) {
  ...
}
if (DTU.getUpdateStrategy() == DomTreeUpdater::UpdateStrategy::Eager) {
  ...
}

After the patch, they can avoid code patterns above

if (DTU.isUpdateLazy()){
  ...
}
if (!DTU.isUpdateLazy()){
  ...
}

[kuhar]

Perhaps it would make sense to expose a similar function for the Eager strategy?
And I would make it sorter, so just isLazy and isEager -- shouldn't make anything more ambiguous IMO.

include/llvm/IR/DomTreeUpdater.h

49 ↗

(On Diff #154509)

Maybe something shorter would also fit here, like:
// Returns true if the current strategy is Lazy.?

[brzycki]

In D49056#1155925, @kuhar wrote:

Perhaps it would make sense to expose a similar function for the Eager strategy?
And I would make it sorter, so just isLazy and isEager -- shouldn't make anything more ambiguous IMO.

+1 to both. Having both options makes it easier to read the if checks in my opinion. Otherwise LGTM.

[NutshellySima]

In D49056#1156730, @brzycki wrote:

In D49056#1155925, @kuhar wrote:

Perhaps it would make sense to expose a similar function for the Eager strategy?
And I would make it sorter, so just isLazy and isEager -- shouldn't make anything more ambiguous IMO.

+1 to both. Having both options makes it easier to read the if checks in my opinion. Otherwise LGTM.

+1. B.T.W, I removed the getUpdateStrategy function because isLazy and isEager can replace its usage.

[NutshellySima]

Address comments.

[kuhar]

Looks like a nice improvement!

llvm/trunk/include/llvm/IR/DomTreeUpdater.h

DomTreeUpdater(DominatorTree &DT_, PostDominatorTree &PDT_,
                  UpdateStrategy Strategy_)
       : DT(&DT_), PDT(&PDT_), Strategy(Strategy_) {}
   DomTreeUpdater(DominatorTree *DT_, PostDominatorTree *PDT_,
                  UpdateStrategy Strategy_)
       : DT(DT_), PDT(PDT_), Strategy(Strategy_) {}
 
   ~DomTreeUpdater() { flush(); }
 
-  /// Returns the UpdateStrategy of the class instance.
-  UpdateStrategy getUpdateStrategy() const { return Strategy; };
+  /// Returns true if the current strategy is Lazy.
+  bool isLazy() const { return Strategy == UpdateStrategy::Lazy; };
+
+  /// Returns true if the current strategy is Eager.
+  bool isEager() const { return Strategy == UpdateStrategy::Eager; };
 
   /// Returns true if it holds a DominatorTree.
   bool hasDomTree() const { return DT != nullptr; }
 
   /// Returns true if it holds a PostDominatorTree.
   bool hasPostDomTree() const { return PDT != nullptr; }
 
   /// Returns true if there is BasicBlock awaiting deletion.

llvm/trunk/unittests/IR/DomTreeUpdaterTest.cpp

TEST(DomTreeUpdater, EagerUpdateBasicOperations) {
 
   // Make the DomTreeUpdater.
   DominatorTree DT(*F);
   PostDominatorTree PDT(*F);
   DomTreeUpdater DTU(DT, PDT, DomTreeUpdater::UpdateStrategy::Eager);
 
   ASSERT_TRUE(DTU.hasDomTree());
   ASSERT_TRUE(DTU.hasPostDomTree());
-  ASSERT_EQ(DTU.getUpdateStrategy(), DomTreeUpdater::UpdateStrategy::Eager);
+  ASSERT_TRUE(DTU.isEager());
+  ASSERT_FALSE(DTU.isLazy());
   ASSERT_TRUE(DTU.getDomTree().verify());
   ASSERT_TRUE(DTU.getPostDomTree().verify());
   ASSERT_FALSE(DTU.hasPendingUpdates());
 
   Function::iterator FI = F->begin();
   BasicBlock *BB0 = &*FI++;
   BasicBlock *BB1 = &*FI++;
   BasicBlock *BB2 = &*FI++;
TEST(DomTreeUpdater, EagerUpdateReplaceEntryBB) {
   Function *F = M->getFunction(FuncName);
 
   // Make the DTU.
   DominatorTree DT(*F);
   PostDominatorTree PDT(*F);
   DomTreeUpdater DTU(DT, PDT, DomTreeUpdater::UpdateStrategy::Eager);
   ASSERT_TRUE(DTU.hasDomTree());
   ASSERT_TRUE(DTU.hasPostDomTree());
-  ASSERT_EQ(DTU.getUpdateStrategy(), DomTreeUpdater::UpdateStrategy::Eager);
+  ASSERT_TRUE(DTU.isEager());
+  ASSERT_FALSE(DTU.isLazy());
   ASSERT_TRUE(DT.verify());
   ASSERT_TRUE(PDT.verify());
 
   Function::iterator FI = F->begin();
   BasicBlock *BB0 = &*FI++;
   BasicBlock *BB1 = &*FI++;
 
   // Add a block as the new function entry BB. We also link it to BB0.
TEST(DomTreeUpdater, LazyUpdateDTBasicOperations) {
   Function *F = M->getFunction(FuncName);
 
   // Make the DTU.
   DominatorTree DT(*F);
   PostDominatorTree *PDT = nullptr;
   DomTreeUpdater DTU(&DT, PDT, DomTreeUpdater::UpdateStrategy::Lazy);
   ASSERT_TRUE(DTU.hasDomTree());
   ASSERT_FALSE(DTU.hasPostDomTree());
-  ASSERT_EQ(DTU.getUpdateStrategy(), DomTreeUpdater::UpdateStrategy::Lazy);
+  ASSERT_FALSE(DTU.isEager());
+  ASSERT_TRUE(DTU.isLazy());
   ASSERT_TRUE(DTU.getDomTree().verify());
 
   Function::iterator FI = F->begin();
   BasicBlock *BB0 = &*FI++;
   BasicBlock *BB1 = &*FI++;
   BasicBlock *BB2 = &*FI++;
   BasicBlock *BB3 = &*FI++;
 
TEST(DomTreeUpdater, LazyUpdateDTInheritedPreds) {
   Function *F = M->getFunction(FuncName);
 
   // Make the DTU.
   DominatorTree DT(*F);
   PostDominatorTree *PDT = nullptr;
   DomTreeUpdater DTU(&DT, PDT, DomTreeUpdater::UpdateStrategy::Lazy);
   ASSERT_TRUE(DTU.hasDomTree());
   ASSERT_FALSE(DTU.hasPostDomTree());
-  ASSERT_EQ(DTU.getUpdateStrategy(), DomTreeUpdater::UpdateStrategy::Lazy);
+  ASSERT_FALSE(DTU.isEager());
+  ASSERT_TRUE(DTU.isLazy());
   ASSERT_TRUE(DTU.getDomTree().verify());
 
   Function::iterator FI = F->begin();
   BasicBlock *BB0 = &*FI++;
   BasicBlock *BB1 = &*FI++;
   BasicBlock *BB2 = &*FI++;
   BasicBlock *BB3 = &*FI++;
 
TEST(DomTreeUpdater, LazyUpdateBasicOperations) {
   Function *F = M->getFunction(FuncName);
 
   // Make the DTU.
   DominatorTree DT(*F);
   PostDominatorTree PDT(*F);
   DomTreeUpdater DTU(&DT, &PDT, DomTreeUpdater::UpdateStrategy::Lazy);
   ASSERT_TRUE(DTU.hasDomTree());
   ASSERT_TRUE(DTU.hasPostDomTree());
-  ASSERT_EQ(DTU.getUpdateStrategy(), DomTreeUpdater::UpdateStrategy::Lazy);
+  ASSERT_FALSE(DTU.isEager());
+  ASSERT_TRUE(DTU.isLazy());
   ASSERT_TRUE(DTU.getDomTree().verify());
   ASSERT_TRUE(DTU.getPostDomTree().verify());
 
   Function::iterator FI = F->begin();
   BasicBlock *BB0 = &*FI++;
   BasicBlock *BB1 = &*FI++;
   BasicBlock *BB2 = &*FI++;
   BasicBlock *BB3 = &*FI++;
TEST(DomTreeUpdater, LazyUpdateReplaceEntryBB) {
   Function *F = M->getFunction(FuncName);
 
   // Make the DTU.
   DominatorTree DT(*F);
   PostDominatorTree PDT(*F);
   DomTreeUpdater DTU(DT, PDT, DomTreeUpdater::UpdateStrategy::Lazy);
   ASSERT_TRUE(DTU.hasDomTree());
   ASSERT_TRUE(DTU.hasPostDomTree());
-  ASSERT_EQ(DTU.getUpdateStrategy(), DomTreeUpdater::UpdateStrategy::Lazy);
+  ASSERT_FALSE(DTU.isEager());
+  ASSERT_TRUE(DTU.isLazy());
   ASSERT_TRUE(DTU.getDomTree().verify());
   ASSERT_TRUE(DTU.getPostDomTree().verify());
 
   Function::iterator FI = F->begin();
   BasicBlock *BB0 = &*FI++;
   BasicBlock *BB1 = &*FI++;
 
   // Add a block as the new function entry BB. We also link it to BB0.
TEST(DomTreeUpdater, LazyUpdateStepTest) {
 
   // Make the DomTreeUpdater.
   DominatorTree DT(*F);
   PostDominatorTree PDT(*F);
   DomTreeUpdater DTU(DT, PDT, DomTreeUpdater::UpdateStrategy::Lazy);
 
   ASSERT_TRUE(DTU.hasDomTree());
   ASSERT_TRUE(DTU.hasPostDomTree());
-  ASSERT_EQ(DTU.getUpdateStrategy(), DomTreeUpdater::UpdateStrategy::Lazy);
+  ASSERT_FALSE(DTU.isEager());
+  ASSERT_TRUE(DTU.isLazy());
   ASSERT_TRUE(DTU.getDomTree().verify());
   ASSERT_TRUE(DTU.getPostDomTree().verify());
   ASSERT_FALSE(DTU.hasPendingUpdates());
 
   Function::iterator FI = F->begin();
   BasicBlock *BB0 = &*FI++;
   FI++;
   BasicBlock *BB2 = &*FI++;