Emit efficient code for switch over strings #11937
Conversation
|
I'm not sure how to make tests that check the backend code generation, in order to assert that we really do end up seeing a |
|
Bytecode tests usually go in https://github.com/lampepfl/dotty/blob/master/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala which makes use of https://github.com/lampepfl/dotty/blob/master/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTest.scala which defines a verifySwitch method you could try to reuse. |
For JS you won't be able to assert that, because in fact Scala.js 1.x has no way to represent switches on Strings in its IR. So they are compiled as if/else chains at the moment. There is an issue upstream to address that (scala-js/scala-js#3843), but it need not concern you here for now. |
| * On a second pass, we emit the switch blocks, one for each different target. | ||
| /* A Match node contains one or more case clauses, each case clause lists one or more | ||
| * Int/String values to use as keys, and a code block. The exception is the "default" case | ||
| * clause which doesn't list any key (there is exactly one of these per match). | ||
| */ | ||
| private def genMatch(tree: Match): BType = tree match { |
There was a problem hiding this comment.
Note that the bytecode this generates ends up having a number of bogus goto,nop,..,nop,athrow sequences in it. Scala 2 doesn't have these because it does some extra dead-code elimination to clean up junk like this (the extra instructions are produced by ClassWriter). I hope to get around to that at some point, but likely not in this PR.
harpocrates
force-pushed
the
alec/string-switch
branch
from
c78020b to
02b0cb4
Compare
|
Hey @harpocrates do you need any more help with this?, a rebase should fix the CI failure |
harpocrates
force-pushed
the
alec/string-switch
branch
from
449eeaa to
3b4ed4b
Compare
The pattern matcher will now emit `Match` with `String` scrutinee as well as the existing `Int` scrutinee. The JVM backend handles this case by emitting bytecode that switches on the String's `hashCode` (this matches what Java does). The SJS already handles `String` matches. The approach is similar to scala/scala#8451 (see scala/bug#11740 too), except that instead of doing a transformation on the AST, we just emit the right bytecode straight away. This is desirable since it means that Scala.js (and any other backend) can choose their own optimised strategy for compiling a match on strings. Fixes scala#11923
harpocrates
force-pushed
the
alec/string-switch
branch
from
3b4ed4b to
3d8fa54
Compare
|
@bishabosha I don't think there is anything else I'm supposed to do - this is just waiting on a review. I just rebased (and flattened the uninteresting history). I did not realize anything was waiting on that - thank you for the ping! |
|
I think it would be nice to include a few sentences about this in the release notes |
The pattern matcher will now emit
MatchwithStringscrutinee aswell as the existing
Intscrutinee. The JVM backend handles this caseby emitting bytecode that switches on the String's
hashCode(thismatches what Java does). The SJS already handles
Stringmatches.The approach is similar to scala/scala#8451 (see scala/bug#11740 too),
except that instead of doing a transformation on the AST, we just emit the
right bytecode straight away. This is desirable since it means that
Scala.js (and any other backend) can choose their own optimised strategy
for compiling a match on strings.
Fixes #11923