[PATCH] D99205: Add jump-threading optimization for deterministic finite automata

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

If we're going to be cloning basic blocks, we need some sort of cost computation. Some blocks aren't legal to clone, and some are expensive simply due to size.  (Illegal to clone like this: convergent, noduplicate, indirectbr predecessors.)  See llvm/Analysis/CodeMetrics.h.

If I'm understanding correctly, the goal here is to specifically handle cases where we can completely eliminate the switch.  That's a bit narrow, but I guess it makes sense, as least as a first cut.



================
Comment at: llvm/lib/CodeGen/DFAJumpThreading.cpp:373
+    if (!FirstDef->getType()->isIntegerTy())
+      return false;
+
----------------
Checking `FirstDef->getType()->isIntegerTy()` isn't necessary, by the definition of SwitchInst.


================
Comment at: llvm/lib/CodeGen/DFAJumpThreading.cpp:453
+    // Vector select instructions are not supported.
+    if (!SI->getCondition()->getType()->isIntegerTy(1)) {
+      return false;
----------------
Don't think you can end up with a vector select here; you've already constrained the result to be an integer.


================
Comment at: llvm/lib/CodeGen/DFAJumpThreading.cpp:566
+  // Key: the parent basic block of that instruction.
+  typedef std::unordered_map<const BasicBlock *, const PHINode *> StateDefMap;
+
----------------
Prefer llvm::DenseMap


================
Comment at: llvm/lib/CodeGen/DFAJumpThreading.cpp:643
+
+        Q.push_back(dyn_cast<PHINode>(Incoming));
+      }
----------------
`cast<PHINode>(Incoming)`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99205/new/

https://reviews.llvm.org/D99205