Merge pull request #14878 from dotty-staging/tailrec-avoid-useless-reassignment-to-this

Two more small improvements to the codegen of tailrec methods

Author: lrytz

compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala

@@ -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. */
@@ -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" +
@@ -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 {

compiler/src/dotty/tools/dotc/transform/TailRec.scala

@@ -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 {

compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala

@@ -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)
     }
   }
 
@@ -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),
       ))
@@ -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),
       ))
     }