Make the label of Return nodes mandatory.
#3323
Conversation
|
After #3322, I felt compelled to try and remove the last asymmetry between Anecdotally, this removes the last |
5093f28 to
dcece86
Compare
Return nodes mandatory.Return nodes mandatory.
|
Rebased. |
A `Return` with a `None` label implicitly meant returning from the
enclosing `Closure` or `MethodDef`. In the spirit of solving all
problems with `Labeled` blocks, we can get rid of this special-case
by introducing an explicit function-wide `Labeled` block when
necessary.
Instead of emitting:
def foo__I(): int = {
...
if (c)
return 5
...
}
we would now use
def foo__I(): int = {
_return[int]: {
...
if (c)
return@_return 5
...
}
}
which is equivalent.
This complicates somewhat the compiler back-end, since it now needs
to take care of adding that label if necessary (it could *always*
add one, but that would not be nice, and defeat most of the
purpose of this change), and it does not really simplify anything
later in the pipeline either.
Moreover, we observe a slight regression of quality of generated
.js code in some rare cases: when there is a `return` in a
`void`-returning method, and that method is not inlined, the
`FunctionEmitter` cannot replace the `break _return` by a `return`
in JS. This is due to a technicality in `FunctionEmitter`, and this
restriction could be lifted in the future.
The main benefits are more regular IR, as well as a potential
performance improvement in the optimizer. Indeed, previously, every
time we inlined a method, we needed to preemptively summon a lot of
complicated machinery to introduce a label in case the body
contains a `return`. In most cases, it doesn't, and we detect after
the fact that the label is useless and can be removed. Instead, we
can now straightforwardly process the body without any extra
action. In the rare cases where the body did contain a `return`,
the optimizer will encounter the `Labeled` block, and spawn the
machinery then. In all the other cases, the machinery is completely
by-passed.
dcece86 to
c61d78d
Compare
|
I like this.
Could you go a bit more into details here? I tried figuring this out, but couldn't (by looking at emitter). |
|
Basically, if we have a def foo__I__V(x: int) {
ret[void]: {
if (x < 0)
return@ret (void 0)
println(x)
}
}When we emit this method, because it is a We could avoid this if, at the top-level of a def, we hijacked a top-level The reason it works better for non-void methods is that we transform the body using Is that clear? |
| @@ -477,13 +473,17 @@ private[emitter] class FunctionEmitter(jsGen: JSGen) { | |||
| val newParams = | |||
| (if (translateRestParam) params.init else params).map(transformParamDef) | |||
|
|
|||
| val newBody = transformStat(withReturn, Set.empty)(env) match { | |||
| val newBody = | |||
| if (isStat) transformStat(body, Set.empty)(env) | |||
There was a problem hiding this comment.
So basically this line would have to hijack the labeled block. Fair enough.
This builds on top of #3322. It will need a rebase, unless we decide to merge both as one in this PR.A
Returnwith aNonelabel implicitly meant returning from the enclosingClosureorMethodDef. In the spirit of solving all problems withLabeledblocks, we can get rid of this special-case by introducing an explicit function-wideLabeledblock when necessary.Instead of emitting:
we would now use
which is equivalent.
This complicates somewhat the compiler back-end, since it now needs to take care of adding that label if necessary (it could always add one, but that would not be nice, and defeat most of the purpose of this change), and it does not really simplify anything later in the pipeline either.
Moreover, we observe a slight regression of quality of generated .js code in some rare cases: when there is a
returnin avoid-returning method, and that method is not inlined, theFunctionEmittercannot replace thebreak _returnby areturnin JS. This is due to a technicality inFunctionEmitter, and this restriction could be lifted in the future.The main benefits are more regular IR, as well as a potential performance improvement in the optimizer. Indeed, previously, every time we inlined a method, we needed to preemptively summon a lot of complicated machinery to introduce a label in case the body contains a
return. In most cases, it doesn't, and we detect after the fact that the label is useless and can be removed. Instead, we can now straightforwardly process the body without any extra action. In the rare cases where the body did contain areturn, the optimizer will encounter theLabeledblock, and spawn the machinery then. In all the other cases, the machinery is completely by-passed.