[PATCH] D60295: [SelectionDAG] Change the jump table size unit from entry to target

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

In D60295#1456414 <https://reviews.llvm.org/D60295#1456414>, @evandro wrote:

> In D60295#1456226 <https://reviews.llvm.org/D60295#1456226>, @hans wrote:
>
> > I also don't see exactly how the semantics of max-jump-table-size are changed with this patch? What am I missing? Can you upload it again with more context maybe?
>
>
> Look at `llvm/include/llvm/CodeGen/TargetLowering.h` below.


Thanks! I had forgotten how this works :-)

I still think looking at the number of cases isn't that much better than looking at the size of the range though. As you said, the point is to limit the load on the branch target predictor, and IIUC that's limited on the number of *different branch targets*, which is really orthogonal to the number of cases. I realize that we don't have that information as readily available, but do you agree that limiting the jump table to a certain number of different targets would be a better approach?



================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:962
+    // the range is dense enough for a jump table.
+    if (NumCases + (NumCases < Range) <= MaxJumpTableCases &&
+        NumCases * 100 >= Range * MinDensity) {
----------------
What's the `(NumCases < Range)` part for? Based on the description, I'd expect this to just check "NumCases <= MaxJumpTableCases"


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60295