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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman 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()) {
----------------
tbaeder wrote:
> 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.
Okay, we can address performance concerns later, if they arise. Thanks!


================
Comment at: clang/test/AST/Interp/switch.cpp:16-18
+  case 11:
+  case 13:
+  case 15:
----------------
tbaeder wrote:
> 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?
The function is called `isEven` and 11, 13, 15 all return true due to fallthrough.


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

https://reviews.llvm.org/D137415