[PATCH] D60295: [CodeGen] Replace -max-jump-table-size with -max-jump-table-targets

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

Very sorry again for the slow reply. I think this patch still suffers from the O(n^3) time complexity.



================
Comment at: llvm/lib/CodeGen/SwitchLoweringUtils.cpp:156
       NumCases = getJumpTableNumCases(TotalCases, i, j);
+      NumTargets = getJumpTableNumTargets(Clusters, i, j);
+      MaxTargets = NumTargets + (HasReachableDefault && Range > NumCases);
----------------
evandro wrote:
> hans wrote:
> > Hmm, this is problematic. getJumpTableNumTargets has linear time complexity in the number of clusters, so I think this is essentially adding another factor O(n) to the overall time complexity, which is unfortunate.
> > 
> > One way to solve this within the original O(n^2) complexity of the current code, would be to build an auxiliary data structure in the outer for-loop, e.g. TotalNumTargets[], where TotalNumTargets[x] is the total number of targets in clusters i..x (i being defined in the outer loop).
> > 
> > That would be built in O(n) time, and the inner loop would then query TotalNumTargets[j] (in constant time).
> > 
> > 
> If I understood you correctly, pre calculating  the number of targets in clusters i..x, where x will be j, is the same as performing this calculation inside the inner loop.
No, the current code does O(n) work in the inner loop, making the overall time complexity O(n^3). Pre-computing the number of targets in clusters i..x in the outer loop would preserve the overall time complexity of O(n^2).


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

https://reviews.llvm.org/D60295