[PATCH] D14122: [SimplifyCFG] Trim duplicate basic blocks in switch cases

[Sanjoy Das via llvm-commits]

sanjoy requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3400
@@ +3399,3 @@
+/// Checks if the bodies of two basic blocks are identical.
+static bool AreBasicBlocksIdentical(BasicBlock &B1, BasicBlock &B2) {
+
----------------
Should this live as `BasicBlock::isIdenticalTo`?

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3403
@@ +3402,3 @@
+  // Two blocks cannot be equivalent if they are different sizes.
+  if (B1.size() != B2.size()) {
+    return false;
----------------
Nit: no braces needed here.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3412
@@ +3411,3 @@
+
+    if (!(*Cur1).isIdenticalTo(&*Cur2)) {
+      return false;
----------------
No braces needed here.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3421
@@ +3420,3 @@
+  // We should've handled this case earlier.
+  assert(Cur1 == B1.end() && Cur2 == B2.end() &&
+      "basics blocks are not the same size");
----------------
I'd change the assert's message to ` We should've handled this case earlier.` and remove the comment.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3458
@@ +3457,3 @@
+    if (AreBasicBlocksIdentical(*CurSuccessor, *NextSuccessor)) {
+        // Throw away one of the successors.
+        I.setSuccessor(CurSuccessor);
----------------
Don't you need to adjust PHI nodes in `CurSuccessor`, since it now has a new incoming block?


Repository:
  rL LLVM

http://reviews.llvm.org/D14122