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

Hans Wennborg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 30 05:52:55 PDT 2019


hans added a comment.

Hi Evandro,

Very sorry for the slow reply here, but it needed some thinking.

>From the change description:

> [CodeGen] Change the jump table size unit from entry to target

It's not so clear what a "jump table size unit" is. I think maybe "Replace -max-jump-table-size with -max-jump-table-targets" or similar would be more clear.



================
Comment at: llvm/include/llvm/CodeGen/SwitchLoweringUtils.h:232
 
+/// Return the number of case targets within a range.
+uint64_t getJumpTableNumTargets(const CaseClusterVector &Clusters,
----------------
Maye add "unique" in here to make it clearer.


================
Comment at: llvm/lib/CodeGen/SwitchLoweringUtils.cpp:51
+    auto C = Clusters[i];
+    Targets.insert(C.MBB);
+  }
----------------
I would probably just have gone with "Targets.insert(Clusters[i].MBB)" since C isn't getting used anywhere else.


================
Comment at: llvm/lib/CodeGen/SwitchLoweringUtils.cpp:96
+  uint64_t NumTargets = getJumpTableNumTargets(Clusters, 0, N - 1);
+  uint64_t MaxTargets = NumTargets + (HasReachableDefault && Range > NumCases);
   assert(NumCases < UINT64_MAX / 100);
----------------
MaxTargets isn't a great variable name for something that's "Number of targets, including the default if it's reachable".

Maybe getJumpTableNumTargets() could take the default case into account so it doesn't need to be handled separately?


================
Comment at: llvm/lib/CodeGen/SwitchLoweringUtils.cpp:156
       NumCases = getJumpTableNumCases(TotalCases, i, j);
+      NumTargets = getJumpTableNumTargets(Clusters, i, j);
+      MaxTargets = NumTargets + (HasReachableDefault && Range > NumCases);
----------------
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).




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

https://reviews.llvm.org/D60295





More information about the llvm-commits mailing list