Skip to content

Commit

Permalink
Merge pull request #14878 from dotty-staging/tailrec-avoid-useless-re…
Browse files Browse the repository at this point in the history
…assignment-to-this

Two more small improvements to the codegen of tailrec methods
  • Loading branch information
lrytz authored Apr 8, 2022
2 parents d673b97 + 18b3deb commit f3cca47
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 56 deletions.
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

0 comments on commit f3cca47

Please # to comment.