[PATCH] D21940: Add support to optionally limit the size of jump tables

[Hans Wennborg via llvm-commits]

hans added inline comments.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8344
@@ -8336,3 +8343,3 @@
     TotalCases[i] = (Hi - Lo).getLimitedValue() + 1;
     if (i != 0)
       TotalCases[i] += TotalCases[i - 1];
----------------
I don't think it can in practice, at least not currently. Each Cluster is created by merging consecutive case-statements with the same destination. So there would have to be an impractically large amount of case-statements to end up with a Cluster where High - Low would overflow.

For the table size however, it's enough to have one "case 0" and one "case UINT64_MAX" to generate a large potential table size. Now, we would obviously never build such a large table, but we would do the size computation, for example in isDense().

================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8353
@@ +8352,3 @@
+  unsigned JumpTableSize = (Clusters[N - 1].High->getValue() -
+                            Clusters[0].Low->getValue())
+                           .getLimitedValue(UINT_MAX - 1) + 1;
----------------
No problem, I think this turned out pretty well too.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8411-8412
@@ -8397,3 +8410,4 @@
         if (NumPartitions < MinPartitions[i] ||
-            (NumPartitions == MinPartitions[i] && Tables > NumTables[i])) {
+            (MaxJumpTableSize == UINT_MAX &&
+             NumPartitions == MinPartitions[i] && Tables > NumTables[i])) {
           MinPartitions[i] = NumPartitions;
----------------
Hmm. I don't like this.

The point of this algorithm is to find jump tables given the size and density criteria. It sounds like your code wants to go the other way.

Also, your code doesn't make us choose the partitioning with the least amount of jump tables, it only removes the code that makes us choose the one with most jump tables, which means we'll end up choosing whichever partitioning that comes first.

So it seems the jt1 function is just getting lucky here.

And in general, picking the partitioning with more jump tables is important here. It's possible to construct a test case with two possible partitionings of equal size, where one results in no jump tables and a very bad lowering.

================
Comment at: llvm/test/CodeGen/Generic/max-jump-table.ll:2
@@ +1,3 @@
+; RUN: llc %s -O2 -print-machineinstrs -march=aarch64 -jump-table-density=40                   -o - 2>%t; FileCheck %s --check-prefixes=CHECK,CHECK0  <%t
+; RUN: llc %s -O2 -print-machineinstrs -march=aarch64 -jump-table-density=40 -max-jump-table=4 -o - 2>%t; FileCheck %s --check-prefixes=CHECK,CHECK4  <%t
+; RUN: llc %s -O2 -print-machineinstrs -march=aarch64 -jump-table-density=40 -max-jump-table=8 -o - 2>%t; FileCheck %s --check-prefixes=CHECK,CHECK8  <%t
----------------
I think the test also needs to move from CodeGen/Generic to CodeGen/AArch64.

The tests under Generic/ will be run in builds which may not have the AArch64 target.


Repository:
  rL LLVM

https://reviews.llvm.org/D21940