[llvm] [SimplifyCFG] Simplify identical predecessors (PR #173022)

[Kunqiu Chen via llvm-commits]

================
@@ -7983,64 +7984,98 @@ static bool simplifySwitchOfCmpIntrinsic(SwitchInst *SI, IRBuilderBase &Builder,
   return true;
 }
 
-/// Checking whether two cases of SI are equal depends on the contents of the
+/// Checking whether two BBs are equal depends on the contents of the
 /// BasicBlock and the incoming values of their successor PHINodes.
 /// PHINode::getIncomingValueForBlock is O(|Preds|), so we'd like to avoid
 /// calling this function on each BasicBlock every time isEqual is called,
 /// especially since the same BasicBlock may be passed as an argument multiple
 /// times. To do this, we can precompute a map of PHINode -> Pred BasicBlock ->
 /// IncomingValue and add it in the Wrapper so isEqual can do O(1) checking
 /// of the incoming values.
-struct SwitchSuccWrapper {
-  BasicBlock *Dest;
-  DenseMap<PHINode *, SmallDenseMap<BasicBlock *, Value *, 8>> *PhiPredIVs;
+struct EqualBBWrapper {
+  BasicBlock *BB;
+
+  // One Phi usually has < 8 incoming values.
+  using BB2ValueMap = SmallDenseMap<BasicBlock *, Value *, 8>;
+  using Phi2IVsMap = DenseMap<PHINode *, BB2ValueMap>;
+  Phi2IVsMap *PhiPredIVs;
+
+  // We only merge the identical non-entry BBs with
+  // - terminator unconditional br to Succ (pending relaxation),
+  // - does not have address taken / weird control.
+  static bool canBeMerged(const BasicBlock *BB) {
+    assert(BB && "Expected non-null BB");
+    // Entry block cannot be eliminated or have predecessors.
+    if (BB->isEntryBlock())
+      return false;
+
+    // Single successor and must be Succ.
+    // FIXME: Relax that the terminator is a BranchInst by checking for equality
+    // on other kinds of terminators. We decide to only support unconditional
+    // branches for now for compile time reasons.
+    auto *BI = dyn_cast<BranchInst>(BB->getTerminator());
+    if (!BI || !BI->isUnconditional())
+      return false;
+
+    // Avoid blocks that are "address-taken" (blockaddress) or have unusual
+    // uses.
+    if (BB->hasAddressTaken() || BB->isLandingPad())
----------------
Camsyn wrote:

Yes. We should use `isEHPad()`.

---

I have added a new test to cover `BB->hasAddressTaken() `; 

---

`BB->isEHPad()` is hard to cover as there are no TWO identical EHPad BB with only ONE unconditioned branch.
I added this guard just for future usage.

https://github.com/llvm/llvm-project/pull/173022