[PATCH] D25212: Add support to tune jump tables

[Evandro Menezes via llvm-commits]

evandro added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8550
+             ((MoreJumpTables && Tables > NumTables[i]) ||
+              (!MoreJumpTables && Tables < NumTables[i])))) {
           MinPartitions[i] = NumPartitions;
----------------
hans wrote:
> evandro wrote:
> > hans wrote:
> > > I really don't like this. The purpose of this algorithm is to find jump tables. If you don't want to find jump tables, don't run it. If you think it's generating too large or sparse jump tables, tweak those parameters.
> > I'd like to have the option to choose more of fewer jump tables when the number of partitions is the same.  It's not clear to me why only one possibility was cast in the code originally, but it doesn't seem to me to be always the best choice.
> >  It's not clear to me why only one possibility was cast in the code originally, but it doesn't seem to me to be always the best choice.
> 
> It's because the whole purpose of the code is to find jump tables.
> 
> Consider the @optimal_jump_table2 test in test/CodeGen/X86/switch.ll. The cases in the switch can be partitioned as {0,1,2,9},{14,15} or {0,1,2},{9,14,15}. The code ensures that the partitioning with the jump table is chosen.
> 
> If you think that's the wrong lowering for your target, maybe increasing the minimum jump table entries is what you want.
Consider that function `jt1` in `test/CodeGen/AArch64/max-jump-table.ll` ends up with partitions {0..7}, {8..12},{13..16} with `-max-jump-table=8`, whereas arguably having its jump table partitioned as {0..7},{8..15} and an extra link in the `if-then-else` chain would be better performing.

I find the hard wired choice of always favoring more jump tables arbitrary.  The algorithm works rather well as is if favoring fewer jump tables.


Repository:
  rL LLVM

https://reviews.llvm.org/D25212