[llvm-commits] [PATCH] [msan] Remove unreachable bb's

Chandler Carruth chandlerc at gmail.com
Fri Dec 14 03:40:23 PST 2012


  My reasoning is simply that SimplifyCFG is *massively* more than you need here.

  It is thousands of lines of CFG simplification and canonicalization. It will make relatively intrusive changes to the CFG that you don't need (or that would already be done before msan is run). This routine is 30-some lines of code, and does exactly one thing.

  Running SimplifyCFG is also a relatively expensive operation, and might not be worth it in all cases.

  Now, I would be quite happy to see logic abstracted out so that SimplifyCFG and this pass could share a 'nuke dead BBs' routine.... But i doubt there is actually much code to share, and so it doesn't seem critical. See my comments inline for some detailed thoughts.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:487-491
@@ +486,7 @@
+    do {
+      BasicBlock *BB = Worklist.pop_back_val();
+      if (!Reachable.insert(BB))
+        continue;
+      for (succ_iterator SI = succ_begin(BB), SE = succ_end(BB); SI != SE; ++SI)
+        Worklist.push_back(*SI);
+    } while (!Worklist.empty());
----------------
In practice, this is significantly more efficient if you hoist the set operation:

  for (succ_iterator SI = succ_begin(BB), SE = succ_end(BB); SI != SE; ++SI)
    if (!Reachable.insert(*SI))
      Worklist.push_back(*SI);

================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:500-504
@@ +499,7 @@
+      if (!Reachable.count(BB)) {
+        for (succ_iterator SI = succ_begin(BB), SE = succ_end(BB); SI != SE; ++SI)
+          if (Reachable.count(*SI))
+            (*SI)->removePredecessor(BB);
+        BB->dropAllReferences();
+        BB = F.getBasicBlockList().erase(BB);
+      } else
----------------
What you want is actually something a bit more similar to DeleteDeadBlock. Sadly you can't just use DeleteDeadBlock because you're trying to handle unreachable cycles.

I would probably lift this out into a wrapper around DeleteDeadBlock which first replaces all uses of the basic block itself with undef (thus clearing its predecessors), and then call DeleteDeadBlock to handle the rest.


http://llvm-reviews.chandlerc.com/D208



More information about the llvm-commits mailing list