[PATCH] D22779: Clone block with icmp+branch if it likely results in further jump threading
Sebastian Pop via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 19 09:02:27 PDT 2016
sebpop added a comment.
I like the idea that this patch is trying to address.
In GCC one can jump-thread across several BBs, whereas this patch "hard-codes" the pattern-matching to only 2 BBs.
It would be good to extend this patch to add a flag that selects the max path length to be jump-threaded.
Then we would need some performance analysis to select a good default for that flag.
The idea is to perform the analysis backwards: one starts from a condition and walk back on the SSA to the definitions involved in that condition.
If on one of the paths the condition can be statically evaluated, then that path is duplicated, the condition is removed, and the final jump is to the destination evaluated at compile time.
This algorithm in turn can be extended to jump-thread across back-edges in loops: that gives about 30% speedup on Coremark, and of course that pattern is present in parsers that implement Finite State Machines (FSM) a loop containing a switch statement, and each case sets the value for the next switch condition in the next iteration. One other example hit by GCC's jump-threading is in libART.
The cost function for code duplication has to be adjusted to account for the full length of the path to be duplicated:
the code in DuplicateBBIntoPreds() evaluates the size of only one BB to be duplicated based on this flag:
cl::desc("Max block size to duplicate for jump threading"),
Now that we copy two BBs their length will be evaluated separately.
I think we need another flag that evaluates the length of the full path to be duplicated.
Comment at: include/llvm/Transforms/Scalar/JumpThreading.h:112
- bool DuplicateCondBranchOnPHIIntoPred(
- BasicBlock *BB, const SmallVectorImpl<BasicBlock *> &PredBBs);
+ bool DuplicateBBIntoPreds(BasicBlock *BB,
+ const SmallVectorImpl<BasicBlock *> &PredBBs);
I see that the other functions in this file do not follow the convention:
now that you are changing the name of this function, please make it start with lower-case.
More information about the llvm-commits