GenBCode: Command-line flags for enabling cleanup optimizations

Add command-line flags `Yopt:...` for simplifying jumps, eliminating
stale line number and label nodes.

`LocalOpt.methodOptimizations` applies all enabled intra-method
optimizations in the right order.

Some cleanups for unreachable code elimination and its tests.

6 files changed, 81 insertions, 85 deletions

diff --git a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
index b892eb36cf..857263d65f 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/CodeGenTools.scala
@@ -7,6 +7,8 @@ import scala.reflect.io.VirtualDirectory
 import scala.tools.asm.Opcodes
 import scala.tools.asm.tree.{AbstractInsnNode, LabelNode, ClassNode, MethodNode}
 import scala.tools.cmd.CommandLineParser
+import scala.tools.nsc.backend.jvm.opt.LocalOpt
+import scala.tools.nsc.settings.{MutableSettings, ScalaSettings}
 import scala.tools.nsc.{Settings, Global}
 import scala.tools.partest.ASMConverters
 import scala.collection.JavaConverters._
@@ -79,4 +81,18 @@ object CodeGenTools {
 
   def getSingleMethod(classNode: ClassNode, name: String): Method =
     convertMethod(classNode.methods.asScala.toList.find(_.name == name).get)
+
+  val localOpt = {
+    val settings = new MutableSettings(msg => throw new IllegalArgumentException(msg))
+    settings.processArguments(List("-Yopt:l:method"), processAll = true)
+    new LocalOpt(settings)
+  }
+
+  import scala.language.implicitConversions
+
+  implicit def aliveInstruction(ins: Instruction): (Instruction, Boolean) = (ins, true)
+
+  implicit class MortalInstruction(val ins: Instruction) extends AnyVal {
+    def dead: (Instruction, Boolean) = (ins, false)
+  }
 }
diff --git a/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala b/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala
index 2fb5bb8052..89900291ca 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/DirectCompileTest.scala
@@ -10,13 +10,12 @@ import scala.tools.partest.ASMConverters._
 
 @RunWith(classOf[JUnit4])
 class DirectCompileTest {
-  val compiler = newCompiler(extraArgs = "-Ybackend:GenBCode")
+  val compiler = newCompiler(extraArgs = "-Ybackend:GenBCode -Yopt:l:method")
 
   @Test
   def testCompile(): Unit = {
     val List(("C.class", bytes)) = compile(compiler)(
-      """
-        |class C {
+      """class C {
         |  def f = 1
         |}
       """.stripMargin)
@@ -26,19 +25,12 @@ class DirectCompileTest {
 
   @Test
   def testCompileClasses(): Unit = {
-    val List(cClass, cModuleClass) = compileClasses(compiler)(
-      """
-        |class C
-        |object C
-      """.stripMargin)
+    val List(cClass, cModuleClass) = compileClasses(compiler)("class C; object C")
 
     assertTrue(cClass.name == "C")
     assertTrue(cModuleClass.name == "C$")
 
-    val List(dMirror, dModuleClass) = compileClasses(compiler)(
-      """
-        |object D
-      """.stripMargin)
+    val List(dMirror, dModuleClass) = compileClasses(compiler)("object D")
 
     assertTrue(dMirror.name == "D")
     assertTrue(dModuleClass.name == "D$")
@@ -47,35 +39,35 @@ class DirectCompileTest {
   @Test
   def testCompileMethods(): Unit = {
     val List(f, g) = compileMethods(compiler)(
-      """
-        |def f = 10
+      """def f = 10
         |def g = f
       """.stripMargin)
     assertTrue(f.name == "f")
     assertTrue(g.name == "g")
 
-    assertTrue(instructionsFromMethod(f).dropNonOp ===
+    assertSameCode(instructionsFromMethod(f).dropNonOp,
       List(IntOp(BIPUSH, 10), Op(IRETURN)))
 
-    assertTrue(instructionsFromMethod(g).dropNonOp ===
-      List(VarOp(ALOAD, 0), Invoke(INVOKEVIRTUAL, "C", "f", "()I", false), Op(IRETURN)))
+    assertSameCode(instructionsFromMethod(g).dropNonOp,
+      List(VarOp(ALOAD, 0), Invoke(INVOKEVIRTUAL, "C", "f", "()I", itf = false), Op(IRETURN)))
   }
 
   @Test
   def testDropNonOpAliveLabels(): Unit = {
+    // makes sure that dropNoOp doesn't drop labels that are being used
     val List(f) = compileMethods(compiler)("""def f(x: Int) = if (x == 0) "a" else "b"""")
-    assertTrue(instructionsFromMethod(f).dropNonOp === List(
-        VarOp(ILOAD, 1),
-        Op(ICONST_0),
-        Jump(IF_ICMPEQ, Label(6)),
-        Jump(GOTO, Label(10)),
-        Label(6),
-        Ldc(LDC, "a"),
-        Jump(GOTO, Label(13)),
-        Label(10),
-        Ldc(LDC, "b"),
-        Label(13),
-        Op(ARETURN)
+    assertSameCode(instructionsFromMethod(f).dropLinesFrames, List(
+      Label(0),
+      VarOp(ILOAD, 1),
+      Op(ICONST_0),
+      Jump(IF_ICMPNE,
+      Label(7)),
+      Ldc(LDC, "a"),
+      Op(ARETURN),
+      Label(7),
+      Ldc(LDC, "b"),
+      Op(ARETURN),
+      Label(11)
     ))
   }
 }
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyExceptionHandlersTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyExceptionHandlersTest.scala
index 57fa1a7b66..7d83c54b5b 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyExceptionHandlersTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyExceptionHandlersTest.scala
@@ -26,7 +26,7 @@ class EmptyExceptionHandlersTest {
       Op(RETURN)
     )
     assertTrue(convertMethod(asmMethod).handlers.length == 1)
-    LocalOpt.removeEmptyExceptionHandlers(asmMethod)
+    localOpt.removeEmptyExceptionHandlers(asmMethod)
     assertTrue(convertMethod(asmMethod).handlers.isEmpty)
   }
 
@@ -35,12 +35,8 @@ class EmptyExceptionHandlersTest {
     val handlers = List(ExceptionHandler(Label(1), Label(2), Label(2), Some(exceptionDescriptor)))
     val asmMethod = genMethod(handlers = handlers)(
       Label(1),          // nops only
-      Op(NOP),
-      Op(NOP),
       Jump(GOTO, Label(3)),
-      Op(NOP),
       Label(3),
-      Op(NOP),
       Jump(GOTO, Label(4)),
 
       Label(2),          // handler
@@ -51,7 +47,7 @@ class EmptyExceptionHandlersTest {
       Op(RETURN)
     )
     assertTrue(convertMethod(asmMethod).handlers.length == 1)
-    LocalOpt.removeEmptyExceptionHandlers(asmMethod)
+    localOpt.removeEmptyExceptionHandlers(asmMethod)
     assertTrue(convertMethod(asmMethod).handlers.isEmpty)
   }
 
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyLabelsAndLineNumbersTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyLabelsAndLineNumbersTest.scala
index 213af4bcc1..8c0168826e 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyLabelsAndLineNumbersTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyLabelsAndLineNumbersTest.scala
@@ -15,8 +15,6 @@ import ASMConverters._
 
 @RunWith(classOf[JUnit4])
 class EmptyLabelsAndLineNumbersTest {
-  import UnreachableCodeTest._ // ".dead" extension method on instructions
-
   @Test
   def removeEmptyLineNumbers(): Unit = {
     val ops = List[(Instruction, Boolean)](
@@ -44,14 +42,14 @@ class EmptyLabelsAndLineNumbersTest {
     )
 
     val method = genMethod()(ops.map(_._1): _*)
-    assertTrue(LocalOpt.removeEmptyLineNumbers(method))
+    assertTrue(localOpt.removeEmptyLineNumbers(method))
     assertSameCode(instructionsFromMethod(method), ops.filter(_._2).map(_._1))
   }
 
   @Test
   def badlyLocatedLineNumbers(): Unit = {
     def t(ops: Instruction*) =
-      assertThrows[AssertionError](LocalOpt.removeEmptyLineNumbers(genMethod()(ops: _*)))
+      assertThrows[AssertionError](localOpt.removeEmptyLineNumbers(genMethod()(ops: _*)))
 
     // line numbers have to be right after their referenced label node
     t(LineNumber(0, Label(1)), Label(1))
@@ -90,7 +88,7 @@ class EmptyLabelsAndLineNumbersTest {
     )
 
     val method = genMethod(handlers = handler)(ops(2, 3, 8, 8, 9, 11).map(_._1): _*)
-    assertTrue(LocalOpt.removeEmptyLabelNodes(method))
+    assertTrue(localOpt.removeEmptyLabelNodes(method))
     val m = convertMethod(method)
     assertSameCode(m.instructions, ops(1, 1, 7, 7, 7, 10).filter(_._2).map(_._1))
     assertTrue(m.handlers match {
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/SimplifyJumpsTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/SimplifyJumpsTest.scala
index a51bce7939..360fa1d23d 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/SimplifyJumpsTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/SimplifyJumpsTest.scala
@@ -26,7 +26,7 @@ class SimplifyJumpsTest {
       Op(RETURN)
     )
     val method = genMethod()(ops: _*)
-    assertTrue(LocalOpt.simplifyJumps(method))
+    assertTrue(localOpt.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), Op(RETURN) :: ops.tail)
   }
 
@@ -45,7 +45,7 @@ class SimplifyJumpsTest {
       Jump(GOTO, Label(2)) :: // replaced by ATHROW
       rest: _*
     )
-    assertTrue(LocalOpt.simplifyJumps(method))
+    assertTrue(localOpt.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), Op(ACONST_NULL) :: Op(ATHROW) :: rest)
   }
 
@@ -66,11 +66,11 @@ class SimplifyJumpsTest {
       Op(RETURN)
     )
     val method = genMethod(handlers = handler)(initialInstrs: _*)
-    assertFalse(LocalOpt.simplifyJumps(method))
+    assertFalse(localOpt.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), initialInstrs)
 
     val optMethod = genMethod()(initialInstrs: _*) // no handler
-    assertTrue(LocalOpt.simplifyJumps(optMethod))
+    assertTrue(localOpt.simplifyJumps(optMethod))
     assertSameCode(instructionsFromMethod(optMethod).take(3), List(Label(1), Op(ACONST_NULL), Op(ATHROW)))
   }
 
@@ -91,7 +91,7 @@ class SimplifyJumpsTest {
       Op(IRETURN)
     )
     val method = genMethod()(begin ::: rest: _*)
-    assertTrue(LocalOpt.simplifyJumps(method))
+    assertTrue(localOpt.simplifyJumps(method))
     assertSameCode(
       instructionsFromMethod(method),
       List(VarOp(ILOAD, 1), Jump(IFLT, Label(3))) ::: rest.tail )
@@ -99,7 +99,7 @@ class SimplifyJumpsTest {
     // no label allowed between begin and rest. if there's another label, then there could be a
     // branch that label. eliminating the  GOTO would change the behavior.
     val nonOptMethod = genMethod()(begin ::: Label(22) :: rest: _*)
-    assertFalse(LocalOpt.simplifyJumps(nonOptMethod))
+    assertFalse(localOpt.simplifyJumps(nonOptMethod))
   }
 
   @Test
@@ -116,7 +116,7 @@ class SimplifyJumpsTest {
     // ensures that the goto is safely removed. ASM supports removing while iterating, but not the
     // next element of the current. Here, the current is the IFGE, the next is the GOTO.
     val method = genMethod()(code(Jump(IFGE, Label(2)), Jump(GOTO, Label(3))): _*)
-    assertTrue(LocalOpt.simplifyJumps(method))
+    assertTrue(localOpt.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), code(Jump(IFLT, Label(3))))
   }
 
@@ -131,7 +131,7 @@ class SimplifyJumpsTest {
       Op(IRETURN)
     )
     val method = genMethod()(ops: _*)
-    assertTrue(LocalOpt.simplifyJumps(method))
+    assertTrue(localOpt.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), ops.tail)
   }
 
@@ -157,7 +157,7 @@ class SimplifyJumpsTest {
       Op(IRETURN)
     )
     val method = genMethod()(ops(1, 2, 3): _*)
-    assertTrue(LocalOpt.simplifyJumps(method))
+    assertTrue(localOpt.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), ops(3, 3, 3))
   }
 
@@ -181,7 +181,7 @@ class SimplifyJumpsTest {
     )
 
     val method = genMethod()(ops(2): _*)
-    assertTrue(LocalOpt.simplifyJumps(method))
+    assertTrue(localOpt.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), ops(3))
   }
 
@@ -202,7 +202,7 @@ class SimplifyJumpsTest {
     )
 
     val method = genMethod()(ops(Jump(IFGE, Label(1))): _*)
-    assertTrue(LocalOpt.simplifyJumps(method))
+    assertTrue(localOpt.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), ops(Op(POP)))
   }
 
@@ -215,7 +215,7 @@ class SimplifyJumpsTest {
       Jump(GOTO, Label(1))
     )
     val method = genMethod()(ops(List(Jump(IF_ICMPGE, Label(1)))): _*)
-    assertTrue(LocalOpt.simplifyJumps(method))
+    assertTrue(localOpt.simplifyJumps(method))
     assertSameCode(instructionsFromMethod(method), ops(List(Op(POP), Op(POP))))
   }
 }
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala
index a3bd7ae6fe..4a45dd9138 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/UnreachableCodeTest.scala
@@ -16,12 +16,20 @@ import ASMConverters._
 
 @RunWith(classOf[JUnit4])
 class UnreachableCodeTest {
-  import UnreachableCodeTest._
+
+  def assertEliminateDead(code: (Instruction, Boolean)*): Unit = {
+    val method = genMethod()(code.map(_._1): _*)
+    localOpt.removeUnreachableCodeImpl(method, "C")
+    val nonEliminated = instructionsFromMethod(method)
+    val expectedLive = code.filter(_._2).map(_._1).toList
+    assertSameCode(nonEliminated, expectedLive)
+  }
 
   // jvm-1.6 enables emitting stack map frames, which impacts the code generation wrt dead basic blocks,
   // see comment in BCodeBodyBuilder
-  val dceCompiler = newCompiler(extraArgs = "-target:jvm-1.6 -Ybackend:GenBCode -Yopt:unreachable-code")
-  val noOptCompiler = newCompiler(extraArgs = "-target:jvm-1.6 -Ybackend:GenBCode -Yopt:l:none")
+  val methodOptCompiler = newCompiler(extraArgs = "-target:jvm-1.6 -Ybackend:GenBCode -Yopt:l:method")
+  val dceCompiler       = newCompiler(extraArgs = "-target:jvm-1.6 -Ybackend:GenBCode -Yopt:unreachable-code")
+  val noOptCompiler     = newCompiler(extraArgs = "-target:jvm-1.6 -Ybackend:GenBCode -Yopt:l:none")
 
   // jvm-1.5 disables computing stack map frames, and it emits dead code as-is.
   val noOptNoFramesCompiler = newCompiler(extraArgs = "-target:jvm-1.5 -Ybackend:GenBCode -Yopt:l:none")
@@ -48,8 +56,8 @@ class UnreachableCodeTest {
   @Test
   def eliminateNop(): Unit = {
     assertEliminateDead(
-      // not dead, since visited by data flow analysis. need a different opt to eliminate it.
-      Op(NOP),
+      // reachable, but removed anyway.
+      Op(NOP).dead,
       Op(RETURN),
       Op(NOP).dead
     )
@@ -136,28 +144,31 @@ class UnreachableCodeTest {
 
   @Test
   def eliminateDeadCatchBlocks(): Unit = {
+    // the Label(1) is live: it's used in the local variable descriptor table (local variable "this" has a range from 0 to 1).
+    def wrapInDefault(code: Instruction*) = List(Label(0), LineNumber(1, Label(0))) ::: code.toList ::: List(Label(1))
+
     val code = "def f: Int = { return 0; try { 1 } catch { case _: Exception => 2 } }"
-    assertSameCode(singleMethodInstructions(dceCompiler)(code).dropNonOp,
-                   List(Op(ICONST_0), Op(IRETURN)))
+    val m = singleMethod(dceCompiler)(code)
+    assertTrue(m.handlers.isEmpty) // redundant (if code is gone, handler is gone), but done once here for extra safety
+    assertSameCode(m.instructions,
+      wrapInDefault(Op(ICONST_0), Op(IRETURN)))
 
     val code2 = "def f: Unit = { try { } catch { case _: Exception => () }; () }"
-    // DCE only removes dead basic blocks, but not NOPs, and also not useless jumps
-    assertSameCode(singleMethodInstructions(dceCompiler)(code2).dropNonOp,
-                   List(Op(NOP), Jump(GOTO, Label(33)), Label(33), Op(RETURN)))
+    // requires fixpoint optimization of methodOptCompiler (dce alone is not enough): first the handler is eliminated, then it's dead catch block.
+    assertSameCode(singleMethodInstructions(methodOptCompiler)(code2), wrapInDefault(Op(RETURN)))
 
     val code3 = "def f: Unit = { try { } catch { case _: Exception => try { } catch { case _: Exception => () } }; () }"
-    assertSameCode(singleMethodInstructions(dceCompiler)(code3).dropNonOp,
-      List(Op(NOP), Jump(GOTO, Label(33)), Label(33), Op(RETURN)))
+    assertSameCode(singleMethodInstructions(methodOptCompiler)(code3), wrapInDefault(Op(RETURN)))
 
+    // this example requires two iterations to get rid of the outer handler.
+    // the first iteration of DCE cannot remove the inner handler. then the inner (empty) handler is removed.
+    // then the second iteration of DCE removes the inner catch block, and then the outer handler is removed.
     val code4 = "def f: Unit = { try { try { } catch { case _: Exception => () } } catch { case _: Exception => () }; () }"
-    assertSameCode(singleMethodInstructions(dceCompiler)(code4).dropNonOp,
-      List(Op(NOP), Jump(GOTO, Label(4)), Label(4), Jump(GOTO, Label(7)), Label(7), Op(RETURN)))
+    assertSameCode(singleMethodInstructions(methodOptCompiler)(code4), wrapInDefault(Op(RETURN)))
   }
 
   @Test // test the dce-testing tools
   def metaTest(): Unit = {
-    assertEliminateDead() // no instructions
-
     assertThrows[AssertionError](
       assertEliminateDead(Op(RETURN).dead),
       _.contains("Expected: List()\nActual  : List(Op(RETURN))")
@@ -198,20 +209,3 @@ class UnreachableCodeTest {
                List(FrameEntry(F_FULL, List(INTEGER, DOUBLE, Label(1)), List("java/lang/Object", Label(3))), Label(1), Label(3)))
   }
 }
-
-object UnreachableCodeTest {
-  import scala.language.implicitConversions
-  implicit def aliveInstruction(ins: Instruction): (Instruction, Boolean) = (ins, true)
-
-  implicit class MortalInstruction(val ins: Instruction) extends AnyVal {
-    def dead: (Instruction, Boolean) = (ins, false)
-  }
-
-  def assertEliminateDead(code: (Instruction, Boolean)*): Unit = {
-    val cls = wrapInClass(genMethod()(code.map(_._1): _*))
-    LocalOpt.removeUnreachableCode(cls)
-    val nonEliminated = instructionsFromMethod(cls.methods.get(0))
-    val expectedLive = code.filter(_._2).map(_._1).toList
-    assertSameCode(nonEliminated, expectedLive)
-  }
-}