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

[Hans Wennborg via Phabricator via llvm-commits]

hans added inline comments.


================
Comment at: llvm/lib/CodeGen/SwitchLoweringUtils.cpp:33
+  PartitionStats Stats;
+  const APInt *Hi, *Lo;
+
----------------
I'd suggest not declaring these until they're used.


================
Comment at: llvm/lib/CodeGen/SwitchLoweringUtils.cpp:36
+  Stats.Cases = 0;
+  // Accumulated instances of unique targets.
+  SmallSet<const MachineBasicBlock *, 8> Targets;
----------------
I don't know what the "accumulated" refers to here. I don't think there's any need for a comment here actually.


================
Comment at: llvm/lib/CodeGen/SwitchLoweringUtils.cpp:39
+  for (unsigned i = First; i <= Last; ++i) {
+    // Accumulate number of cases in the range of clusters.
+    Hi = &Clusters[i].High->getValue();
----------------
There's nothing accumulated here. I'd suggest just dropping the comment.


================
Comment at: llvm/lib/CodeGen/SwitchLoweringUtils.cpp:41
+    Hi = &Clusters[i].High->getValue();
+    Lo = &Clusters[i].Low->getValue();
+    Stats.Cases += (*Hi - *Lo).getLimitedValue() + 1;
----------------
It would be better to just declare the variables here:

APInt *Hi = ...
APInt *Lo = ...


================
Comment at: llvm/lib/CodeGen/SwitchLoweringUtils.cpp:49
+  Hi = &Clusters[Last].High->getValue();
+  Lo = &Clusters[First].Low->getValue();
+  assert(Hi->getBitWidth() == Lo->getBitWidth());
----------------
And declare these variables here (no need to reuse the same variables as in the loop):

APInt *Hi = ...
APInt * Lo = ...


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

https://reviews.llvm.org/D60295