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

[Alexey Zhikhartsev via Phabricator via llvm-commits]

alexey.zhikhar marked 3 inline comments as done.
alexey.zhikhar added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp:78
+static cl::opt<unsigned> MaxPathDepth(
+    "dfa-max-path-length",
+    cl::desc("Max number of blocks searched to find threadable path"),
----------------
alexey.zhikhar wrote:
> SjoerdMeijer wrote:
> > I think this option is not tested.
> A test case for `MaxPathDepth` is not ready yet, but duly noted. Thanks
Please see `max_path_length`


================
Comment at: llvm/lib/Transforms/Scalar/DFAJumpThreading.cpp:722
+    unsigned CondBranches =
+        APInt(32, Switch->getNumSuccessors()).ceilLogBase2();
+    unsigned NumInsts = Metrics.NumInsts;
----------------
ChuanqiXu wrote:
> alexey.zhikhar wrote:
> > ChuanqiXu wrote:
> > > What's the rationale to use ceil log base 2? Maybe  there is a conclusion in math. But I guess it may be better to tell it.
> > Added a comment.
> Is this assumption stable? For the one hand, the motivation example in Coremark wouldn't be lowered Into a binary tree. On the other hand, it may be possible that the switch statement may be lowered to a jump table.
That's a good point, I added a separate clause for when a jump table is expected instead of binary search (the same hook is used in inlining).


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

https://reviews.llvm.org/D99205