[PATCH] D137415: [clang][Interp] Implement switch statements

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:423
+  // Create labels and comparison ops for all case statements.
+  for (const SwitchCase *SC = S->getSwitchCaseList(); SC;
+       SC = SC->getNextSwitchCase()) {
----------------
aaron.ballman wrote:
> How well does this handle large switch statements? Generated code sometimes winds up with thousands of cases, so I'm wondering if we need to have a jump table implementation for numerous cases and use the simple comparison implementation when there's only a small number of labels (and we can test to see what constitutes a good value for "small number of labels").
> 
> Also, one thing to be aware of (that hopefully won't matter TOO much in this case): the switch case list does not have a stable iteration order IIRC.
With 10'000 case labels, it seems to be slightly slower if all case labels need to be iterated through and slightly faster if the first iteration already hits. I'm not too concerned about the performance here tbh.


================
Comment at: clang/test/AST/Interp/switch.cpp:16-18
+  case 11:
+  case 13:
+  case 15:
----------------
aaron.ballman wrote:
> lol, you should probably fix this so it's not so confusing to casual readers.
What exactly is the confusing part? :D That the case labels are out of order?


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

https://reviews.llvm.org/D137415