Merge pull request #753 from WebAssembly/cfg-opts

Minor cfg-traversal opts

2 files changed, 40 insertions, 20 deletions

diff --git a/src/cfg/cfg-traversal.h b/src/cfg/cfg-traversal.h
index 5bc593691..99bc25ea8 100644
--- a/src/cfg/cfg-traversal.h
+++ b/src/cfg/cfg-traversal.h
@@ -56,17 +56,29 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
   std::vector<std::unique_ptr<BasicBlock>> basicBlocks; // all the blocks
 
   // traversal state
-  BasicBlock* currBasicBlock; // the current block in play during traversal
+  BasicBlock* currBasicBlock; // the current block in play during traversal. can be nullptr if unreachable,
+                              // but note that we don't do a deep unreachability analysis - just enough
+                              // to avoid constructing obviously-unreachable blocks (we do a full reachability
+                              // analysis on the CFG once it is constructed).
   std::map<Expression*, std::vector<BasicBlock*>> branches; // a block or loop => its branches
   std::vector<BasicBlock*> ifStack;
   std::vector<BasicBlock*> loopStack;
 
-  static void doStartBasicBlock(SubType* self, Expression** currp) {
-    self->currBasicBlock = self->makeBasicBlock();
-    self->basicBlocks.push_back(std::unique_ptr<BasicBlock>(self->currBasicBlock));
+  void startBasicBlock() {
+    currBasicBlock = makeBasicBlock();
+    basicBlocks.push_back(std::unique_ptr<BasicBlock>(currBasicBlock));
+  }
+
+  void startUnreachableBlock() {
+    currBasicBlock = nullptr;
+  }
+
+  static void doStartUnreachableBlock(SubType* self, Expression** currp) {
+    self->startUnreachableBlock();
   }
 
   void link(BasicBlock* from, BasicBlock* to) {
+    if (!from || !to) return; // if one of them is not reachable, ignore
     from->out.push_back(to);
     to->in.push_back(from);
   }
@@ -80,7 +92,7 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
     if (origins.size() == 0) return;
     // we have branches to here, so we need a new block
     auto* last = self->currBasicBlock;
-    doStartBasicBlock(self, currp);
+    self->startBasicBlock();
     self->link(last, self->currBasicBlock); // fallthrough
     // branches to the new one
     for (auto* origin : origins) {
@@ -91,20 +103,20 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
 
   static void doStartIfTrue(SubType* self, Expression** currp) {
     auto* last = self->currBasicBlock;
-    doStartBasicBlock(self, currp);
+    self->startBasicBlock();
     self->link(last, self->currBasicBlock); // ifTrue
     self->ifStack.push_back(last); // the block before the ifTrue
   }
 
   static void doStartIfFalse(SubType* self, Expression** currp) {
     self->ifStack.push_back(self->currBasicBlock); // the ifTrue fallthrough
-    doStartBasicBlock(self, currp);
+    self->startBasicBlock();
     self->link(self->ifStack[self->ifStack.size() - 2], self->currBasicBlock); // before if -> ifFalse
   }
 
   static void doEndIf(SubType* self, Expression** currp) {
     auto* last = self->currBasicBlock;
-    doStartBasicBlock(self, currp);
+    self->startBasicBlock();
     self->link(last, self->currBasicBlock); // last one is ifFalse's fallthrough if there was one, otherwise it's the ifTrue fallthrough
     if ((*currp)->cast<If>()->ifFalse) {
       // we just linked ifFalse, need to link ifTrue to the end
@@ -119,14 +131,14 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
 
   static void doStartLoop(SubType* self, Expression** currp) {
     auto* last = self->currBasicBlock;
-    doStartBasicBlock(self, currp);
+    self->startBasicBlock();
     self->link(last, self->currBasicBlock);
     self->loopStack.push_back(self->currBasicBlock);
   }
 
   static void doEndLoop(SubType* self, Expression** currp) {
     auto* last = self->currBasicBlock;
-    doStartBasicBlock(self, currp);
+    self->startBasicBlock();
     self->link(last, self->currBasicBlock); // fallthrough
     auto* curr = (*currp)->cast<Loop>();
     // branches to the top of the loop
@@ -144,10 +156,12 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
   static void doEndBreak(SubType* self, Expression** currp) {
     auto* curr = (*currp)->cast<Break>();
     self->branches[self->findBreakTarget(curr->name)].push_back(self->currBasicBlock); // branch to the target
-    auto* last = self->currBasicBlock;
-    doStartBasicBlock(self, currp);
     if (curr->condition) {
+      auto* last = self->currBasicBlock;
+      self->startBasicBlock();
       self->link(last, self->currBasicBlock); // we might fall through
+    } else {
+      self->startUnreachableBlock();
     }
   }
 
@@ -163,7 +177,7 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
     if (!seen.count(curr->default_)) {
       self->branches[self->findBreakTarget(curr->default_)].push_back(self->currBasicBlock); // branch to the target
     }
-    doStartBasicBlock(self, currp);
+    self->startUnreachableBlock();
   }
 
   static void scan(SubType* self, Expression** currp) {
@@ -175,9 +189,7 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
         break;
       }
       case Expression::Id::IfId: {
-        self->pushTask(SubType::doPostVisitControlFlow, currp);
         self->pushTask(SubType::doEndIf, currp);
-        self->pushTask(SubType::doVisitIf, currp);
         auto* ifFalse = curr->cast<If>()->ifFalse;
         if (ifFalse) {
           self->pushTask(SubType::scan, &curr->cast<If>()->ifFalse);
@@ -186,7 +198,6 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
         self->pushTask(SubType::scan, &curr->cast<If>()->ifTrue);
         self->pushTask(SubType::doStartIfTrue, currp);
         self->pushTask(SubType::scan, &curr->cast<If>()->condition);
-        self->pushTask(SubType::doPreVisitControlFlow, currp);
         return; // don't do anything else
       }
       case Expression::Id::LoopId: {
@@ -202,11 +213,11 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
         break;
       }
       case Expression::Id::ReturnId: {
-        self->pushTask(SubType::doStartBasicBlock, currp);
+        self->pushTask(SubType::doStartUnreachableBlock, currp);
         break;
       }
       case Expression::Id::UnreachableId: {
-        self->pushTask(SubType::doStartBasicBlock, currp);
+        self->pushTask(SubType::doStartUnreachableBlock, currp);
         break;
       }
       default: {}
@@ -226,7 +237,7 @@ struct CFGWalker : public ControlFlowWalker<SubType, VisitorType> {
   void doWalkFunction(Function* func) {
     basicBlocks.clear();
 
-    doStartBasicBlock(static_cast<SubType*>(this), nullptr);
+    startBasicBlock();
     entry = currBasicBlock;
     ControlFlowWalker<SubType, VisitorType>::doWalkFunction(func);
 
diff --git a/src/passes/CoalesceLocals.cpp b/src/passes/CoalesceLocals.cpp
index ba2c6dd81..2ff1dde37 100644
--- a/src/passes/CoalesceLocals.cpp
+++ b/src/passes/CoalesceLocals.cpp
@@ -169,13 +169,22 @@ struct CoalesceLocals : public WalkerPass<CFGWalker<CoalesceLocals, Visitor<Coal
 
   static void doVisitGetLocal(CoalesceLocals* self, Expression** currp) {
     auto* curr = (*currp)->cast<GetLocal>();
+     // if in unreachable code, ignore
+    if (!self->currBasicBlock) {
+      ExpressionManipulator::convert<GetLocal, Unreachable>(curr);
+      return;
+    }
     self->currBasicBlock->contents.actions.emplace_back(Action::Get, curr->index, currp);
   }
 
   static void doVisitSetLocal(CoalesceLocals* self, Expression** currp) {
     auto* curr = (*currp)->cast<SetLocal>();
+    // if in unreachable code, ignore
+    if (!self->currBasicBlock) {
+      ExpressionManipulator::nop(curr);
+      return;
+    }
     self->currBasicBlock->contents.actions.emplace_back(Action::Set, curr->index, currp);
-
     // if this is a copy, note it
     auto* get = curr->value->dynCast<GetLocal>();
     if (get) self->addCopy(curr->index, get->index);