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.

4 files changed, 44 insertions, 56 deletions

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