Skip to content

Two more small improvements to the codegen of tailrec methods #14878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 40 additions & 35 deletions compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -113,16 +113,14 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
}
}

def genThrow(expr: Tree): BType = {
def genThrow(expr: Tree): Unit = {
val thrownKind = tpeTK(expr)
// `throw null` is valid although scala.Null (as defined in src/libray-aux) isn't a subtype of Throwable.
// Similarly for scala.Nothing (again, as defined in src/libray-aux).
assert(thrownKind.isNullType || thrownKind.isNothingType || thrownKind.asClassBType.isSubtypeOf(ThrowableReference))
genLoad(expr, thrownKind)
lineNumber(expr)
emit(asm.Opcodes.ATHROW) // ICode enters here into enterIgnoreMode, we'll rely instead on DCE at ClassNode level.

RT_NOTHING // always returns the same, the invoker should know :)
}

/* Generate code for primitive arithmetic operations. */
Expand Down Expand Up @@ -319,13 +317,14 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
generatedType = expectedType

case t @ WhileDo(_, _) =>
generatedType = genWhileDo(t)
generatedType = genWhileDo(t, expectedType)

case t @ Try(_, _, _) =>
generatedType = genLoadTry(t)

case t: Apply if t.fun.symbol eq defn.throwMethod =>
generatedType = genThrow(t.args.head)
genThrow(t.args.head)
generatedType = expectedType

case New(tpt) =>
abort(s"Unexpected New(${tpt.tpe.showSummary()}/$tpt) reached GenBCode.\n" +
Expand Down Expand Up @@ -586,41 +585,47 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
}
} // end of genReturn()

def genWhileDo(tree: WhileDo): BType = tree match{
def genWhileDo(tree: WhileDo, expectedType: BType): BType = tree match{
case WhileDo(cond, body) =>

val isInfinite = cond == tpd.EmptyTree

val loop = new asm.Label
markProgramPoint(loop)

if (isInfinite) {
genLoad(body, UNIT)
bc goTo loop
RT_NOTHING
} else {
val hasBody = cond match {
case Literal(value) if value.tag == UnitTag => false
case _ => true
}

if (hasBody) {
val success = new asm.Label
val failure = new asm.Label
genCond(cond, success, failure, targetIfNoJump = success)
markProgramPoint(success)
genLoad(body, UNIT)
bc goTo loop
markProgramPoint(failure)
} else {
// this is the shape of do..while loops, so do something smart about them
val failure = new asm.Label
genCond(cond, loop, failure, targetIfNoJump = failure)
markProgramPoint(failure)
}

if isInfinite then
body match
case Labeled(bind, expr) if tpeTK(body) == UNIT =>
// this is the shape of tailrec methods
val loop = programPoint(bind.symbol)
markProgramPoint(loop)
genLoad(expr, UNIT)
bc goTo loop
case _ =>
val loop = new asm.Label
markProgramPoint(loop)
genLoad(body, UNIT)
bc goTo loop
end match
expectedType
else
body match
case Literal(value) if value.tag == UnitTag =>
// this is the shape of do..while loops
val loop = new asm.Label
markProgramPoint(loop)
val exitLoop = new asm.Label
genCond(cond, loop, exitLoop, targetIfNoJump = exitLoop)
markProgramPoint(exitLoop)
case _ =>
val loop = new asm.Label
val success = new asm.Label
val failure = new asm.Label
markProgramPoint(loop)
genCond(cond, success, failure, targetIfNoJump = success)
markProgramPoint(success)
genLoad(body, UNIT)
bc goTo loop
markProgramPoint(failure)
end match
UNIT
}
}

def genTypeApply(t: TypeApply): BType = (t: @unchecked) match {
Expand Down
11 changes: 7 additions & 4 deletions compiler/src/dotty/tools/dotc/transform/TailRec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -349,10 +349,13 @@ class TailRec extends MiniPhase {
yield
(getVarForRewrittenParam(param), arg)

val assignThisAndParamPairs =
if (prefix eq EmptyTree) assignParamPairs
else
// TODO Opt: also avoid assigning `this` if the prefix is `this.`
val assignThisAndParamPairs = prefix match
case EmptyTree =>
assignParamPairs
case prefix: This if prefix.symbol == enclosingClass =>
// Avoid assigning `this = this`
assignParamPairs
case _ =>
(getVarForRewrittenThis(), noTailTransform(prefix)) :: assignParamPairs

val assignments = assignThisAndParamPairs match {
Expand Down
30 changes: 13 additions & 17 deletions compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ class TestBCode extends DottyBytecodeTest {
val clsIn = dir.lookupName("Test.class", directory = false).input
val clsNode = loadClassNode(clsIn)
val method = getMethod(clsNode, "test")
assertEquals(94, instructionsFromMethod(method).size)
assertEquals(93, instructionsFromMethod(method).size)
}
}

Expand Down Expand Up @@ -976,29 +976,25 @@ class TestBCode extends DottyBytecodeTest {
Op(ICONST_0),
Jump(IF_ICMPNE, Label(7)),
VarOp(ILOAD, 2),
Jump(GOTO, Label(26)),
Jump(GOTO, Label(22)),
Label(7),
VarOp(ALOAD, 0),
VarOp(ASTORE, 3),
VarOp(ILOAD, 1),
Op(ICONST_1),
Op(ISUB),
VarOp(ISTORE, 4),
VarOp(ISTORE, 3),
VarOp(ILOAD, 2),
VarOp(ILOAD, 1),
Op(IMUL),
VarOp(ISTORE, 5),
VarOp(ALOAD, 3),
VarOp(ASTORE, 0),
VarOp(ILOAD, 4),
VarOp(ISTORE, 4),
VarOp(ILOAD, 3),
VarOp(ISTORE, 1),
VarOp(ILOAD, 5),
VarOp(ILOAD, 4),
VarOp(ISTORE, 2),
Jump(GOTO, Label(29)),
Label(26),
Op(IRETURN),
Label(29),
Jump(GOTO, Label(0)),
Label(22),
Op(IRETURN),
Op(NOP),
Op(NOP),
Op(NOP),
Op(ATHROW),
))
Expand Down Expand Up @@ -1032,12 +1028,12 @@ class TestBCode extends DottyBytecodeTest {
VarOp(ASTORE, 0),
VarOp(ILOAD, 4),
VarOp(ISTORE, 1),
Jump(GOTO, Label(29)),
Jump(GOTO, Label(0)),
Label(26),
Op(IRETURN),
Label(29),
Jump(GOTO, Label(0)),
Op(NOP),
Op(NOP),
Op(ATHROW),
Op(ATHROW),
))
}
Expand Down