Merge pull request #4088 from lrytz/opt/cleanupOptimizations

GenBCode: cleanup optimizations

8 files changed, 554 insertions, 68 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..c1c5a71b83 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,23 @@ object CodeGenTools {
 
   def getSingleMethod(classNode: ClassNode, name: String): Method =
     convertMethod(classNode.methods.asScala.toList.find(_.name == name).get)
+
+  def assertHandlerLabelPostions(h: ExceptionHandler, instructions: List[Instruction], startIndex: Int, endIndex: Int, handlerIndex: Int): Unit = {
+    val insVec = instructions.toVector
+    assertTrue(h.start == insVec(startIndex) && h.end == insVec(endIndex) && h.handler == insVec(handlerIndex))
+  }
+
+  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/CompactLocalVariablesTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/CompactLocalVariablesTest.scala
new file mode 100644
index 0000000000..fc748196d0
--- /dev/null
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/CompactLocalVariablesTest.scala
@@ -0,0 +1,80 @@
+package scala.tools.nsc
+package backend.jvm
+package opt
+
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.junit.Test
+import scala.tools.asm.Opcodes._
+import org.junit.Assert._
+
+import CodeGenTools._
+import scala.tools.partest.ASMConverters
+import ASMConverters._
+
+@RunWith(classOf[JUnit4])
+class CompactLocalVariablesTest {
+
+  // recurse-unreachable-jumps is required for eliminating catch blocks, in the first dce round they
+  // are still live.only after eliminating the empty handler the catch blocks become unreachable.
+  val methodOptCompiler     = newCompiler(extraArgs = "-target:jvm-1.6 -Ybackend:GenBCode -Yopt:unreachable-code,recurse-unreachable-jumps,compact-locals")
+  val noCompactVarsCompiler = newCompiler(extraArgs = "-target:jvm-1.6 -Ybackend:GenBCode -Yopt:unreachable-code,recurse-unreachable-jumps")
+
+  @Test
+  def compactUnused(): Unit = {
+    val code =
+      """def f: Double = {
+        |  try { }
+        |  catch {
+        |    case _: Throwable =>
+        |      // eliminated by dce
+        |      val i = 1
+        |      val d = 1d
+        |      val f = 1f
+        |      val l = 1l
+        |  }
+        |
+        |  val i = 1      // variable index 1 (it's an instance method, so at index 0 we have `this`)
+        |  val d = 1d     // 2,3
+        |  val f = 1f     // 4
+        |  val l = 1l     // 5,6
+        |
+        |  try { }
+        |  catch {
+        |    case _: Throwable =>
+        |      // eliminated by dce
+        |      val i = 1
+        |      val d = 1d
+        |      val f = 1f
+        |      val l = 1l
+        |  }
+        |
+        |  val ii = 1     // 7
+        |  val dd = 1d    // 8,9
+        |  val ff = 1f    // 10
+        |  val ll = 1l    // 11,12
+        |
+        |  i + ii + d + dd + f + ff + l + ll
+        |}
+        |""".stripMargin
+
+    val List(noCompact)   = compileMethods(noCompactVarsCompiler)(code)
+    val List(withCompact) = compileMethods(methodOptCompiler)(code)
+
+    // code is the same, except for local var indices
+    assertTrue(noCompact.instructions.size == withCompact.instructions.size)
+
+    val varOpSlots = convertMethod(withCompact).instructions collect {
+      case VarOp(_, v) => v
+    }
+    assertTrue(varOpSlots.toString, varOpSlots == List(1, 2, 4, 5, 7, 8, 10, 11,  // stores
+                                                       1, 7, 2, 8, 4, 10, 5, 11)) // loads
+
+    // the local variables descriptor table is cleaned up to remove stale entries after dce,
+    // also when the slots are not compacted
+    assertTrue(noCompact.localVariables.size == withCompact.localVariables.size)
+
+    assertTrue(noCompact.maxLocals == 25)
+    assertTrue(withCompact.maxLocals == 13)
+  }
+}
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
new file mode 100644
index 0000000000..8c0168826e
--- /dev/null
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/EmptyLabelsAndLineNumbersTest.scala
@@ -0,0 +1,99 @@
+package scala.tools.nsc
+package backend.jvm
+package opt
+
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.junit.Test
+import scala.tools.asm.Opcodes._
+import org.junit.Assert._
+import scala.tools.testing.AssertUtil._
+
+import CodeGenTools._
+import scala.tools.partest.ASMConverters
+import ASMConverters._
+
+@RunWith(classOf[JUnit4])
+class EmptyLabelsAndLineNumbersTest {
+  @Test
+  def removeEmptyLineNumbers(): Unit = {
+    val ops = List[(Instruction, Boolean)](
+      Label(1),
+      LineNumber(1, Label(1)),
+      Label(2),
+      Label(3),
+      Op(RETURN),
+
+      Label(4),
+      LineNumber(4, Label(4)).dead,
+      LineNumber(5, Label(4)),
+      Op(RETURN),
+
+      Label(5),
+      LineNumber(6, Label(5)).dead,
+      Label(6),
+      Label(7),
+      LineNumber(7, Label(7)),
+      Op(RETURN),
+
+      Label(9),
+      LineNumber(8, Label(9)).dead,
+      Label(10)
+    )
+
+    val method = genMethod()(ops.map(_._1): _*)
+    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: _*)))
+
+    // line numbers have to be right after their referenced label node
+    t(LineNumber(0, Label(1)), Label(1))
+    t(Label(0), Label(1), LineNumber(0, Label(0)))
+  }
+
+  @Test
+  def removeEmptyLabels(): Unit = {
+    val handler = List(ExceptionHandler(Label(4), Label(5), Label(6), Some("java/lang/Throwable")))
+    def ops(target1: Int, target2: Int, target3: Int, target4: Int, target5: Int, target6: Int) = List[(Instruction, Boolean)](
+      Label(1),
+      Label(2).dead,
+      Label(3).dead,
+      LineNumber(3, Label(target1)),
+      VarOp(ILOAD, 1),
+      Jump(IFGE, Label(target2)),
+
+      Label(4),
+      Label(5).dead,
+      Label(6).dead,
+      VarOp(ILOAD, 2),
+      Jump(IFGE, Label(target3)),
+
+      Label(7),
+      Label(8).dead,
+      Label(9).dead,
+      Op(RETURN),
+
+      LookupSwitch(LOOKUPSWITCH, Label(target4), List(1,2), List(Label(target4), Label(target5))),
+      TableSwitch(TABLESWITCH, 1, 2, Label(target4), List(Label(target4), Label(target5))),
+
+      Label(10),
+      LineNumber(10, Label(10)),
+      Label(11).dead,
+      LineNumber(12, Label(target6))
+    )
+
+    val method = genMethod(handlers = handler)(ops(2, 3, 8, 8, 9, 11).map(_._1): _*)
+    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 {
+      case List(ExceptionHandler(Label(4), Label(4), Label(4), _)) => true
+      case _ => false
+    })
+  }
+}
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/MethodLevelOpts.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/MethodLevelOpts.scala
new file mode 100644
index 0000000000..5b0f0f238a
--- /dev/null
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/MethodLevelOpts.scala
@@ -0,0 +1,83 @@
+package scala.tools.nsc
+package backend.jvm
+package opt
+
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.junit.Test
+import scala.tools.asm.Opcodes._
+import org.junit.Assert._
+
+import scala.tools.testing.AssertUtil._
+
+import CodeGenTools._
+import scala.tools.partest.ASMConverters
+import ASMConverters._
+
+@RunWith(classOf[JUnit4])
+class MethodLevelOpts {
+  val methodOptCompiler = newCompiler(extraArgs = "-target:jvm-1.6 -Ybackend:GenBCode -Yopt:l:method")
+
+  def wrapInDefault(code: Instruction*) = List(Label(0), LineNumber(1, Label(0))) ::: code.toList ::: List(Label(1))
+
+  @Test
+  def eliminateEmptyTry(): Unit = {
+    val code = "def f = { try {} catch { case _: Throwable => 0; () }; 1 }"
+    assertSameCode(singleMethodInstructions(methodOptCompiler)(code), wrapInDefault(Op(ICONST_1), Op(IRETURN)))
+  }
+
+  @Test
+  def cannotEliminateLoadBoxedUnit(): Unit = {
+    // the compiler inserts a boxed into the try block. it's therefore non-empty (and live) and not eliminated.
+    val code = "def f = { try {} catch { case _: Throwable => 0 }; 1 }"
+    val m = singleMethod(methodOptCompiler)(code)
+    assertTrue(m.handlers.length == 1)
+    assertSameCode(m.instructions.take(3), List(Label(0), LineNumber(1, Label(0)), Field(GETSTATIC, "scala/runtime/BoxedUnit", "UNIT", "Lscala/runtime/BoxedUnit;")))
+  }
+
+  @Test
+  def inlineThrowInCatchNotTry(): Unit = {
+    // the try block does not contain the `ATHROW` instruction, but in the catch block, `ATHROW` is inlined
+    val code = "def f(e: Exception) = throw { try e catch { case _: Throwable => e } }"
+    val m = singleMethod(methodOptCompiler)(code)
+    assertHandlerLabelPostions(m.handlers.head, m.instructions, 0, 3, 5)
+    assertSameCode(m.instructions,
+      wrapInDefault(VarOp(ALOAD, 1), Label(3), Op(ATHROW), Label(5), FrameEntry(4, List(), List("java/lang/Throwable")), Op(POP), VarOp(ALOAD, 1), Op(ATHROW))
+    )
+  }
+
+  @Test
+  def inlineReturnInCachtNotTry(): Unit = {
+    val code = "def f: Int = return { try 1 catch { case _: Throwable => 2 } }"
+    // cannot inline the IRETURN into the try block (because RETURN may throw IllegalMonitorState)
+    val m = singleMethod(methodOptCompiler)(code)
+    assertHandlerLabelPostions(m.handlers.head, m.instructions, 0, 3, 5)
+    assertSameCode(m.instructions,
+      wrapInDefault(Op(ICONST_1), Label(3), Op(IRETURN), Label(5), FrameEntry(4, List(), List("java/lang/Throwable")), Op(POP), Op(ICONST_2), Op(IRETURN)))
+  }
+
+  @Test
+  def simplifyJumpsInTryCatchFinally(): Unit = {
+    val code =
+      """def f: Int =
+        |  try {
+        |    return 1
+        |  } catch {
+        |    case _: Throwable =>
+        |    return 2
+        |  } finally {
+        |    return 2
+        |    // dead
+        |    val x = try 10 catch { case _: Throwable => 11 }
+        |    println(x)
+        |  }
+      """.stripMargin
+    val m = singleMethod(methodOptCompiler)(code)
+    assertTrue(m.handlers.length == 2)
+    assertSameCode(m.instructions.dropNonOp, // drop line numbers and lables that are only used by line numbers
+
+      // one single label left :-)
+      List(Op(ICONST_1), VarOp(ISTORE, 2), Jump(GOTO, Label(20)), Op(POP), Op(ICONST_2), VarOp(ISTORE, 2), Jump(GOTO, Label(20)), VarOp(ASTORE, 3), Op(ICONST_2), Op(IRETURN), Label(20), Op(ICONST_2), Op(IRETURN))
+    )
+  }
+}
diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/SimplifyJumpsTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/SimplifyJumpsTest.scala
new file mode 100644
index 0000000000..360fa1d23d
--- /dev/null
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/SimplifyJumpsTest.scala
@@ -0,0 +1,221 @@
+package scala.tools.nsc
+package backend.jvm
+package opt
+
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.junit.Test
+import scala.tools.asm.Opcodes._
+import org.junit.Assert._
+
+import CodeGenTools._
+import scala.tools.partest.ASMConverters
+import ASMConverters._
+
+@RunWith(classOf[JUnit4])
+class SimplifyJumpsTest {
+  @Test
+  def simpleGotoReturn(): Unit = {
+    val ops = List(
+      Jump(GOTO, Label(2)), // replaced by RETURN
+      Op(ICONST_1),         // need some code, otherwise removeJumpToSuccessor kicks in
+      Op(POP),
+      Label(1),             // multiple labels OK
+      Label(2),
+      Label(3),
+      Op(RETURN)
+    )
+    val method = genMethod()(ops: _*)
+    assertTrue(localOpt.simplifyJumps(method))
+    assertSameCode(instructionsFromMethod(method), Op(RETURN) :: ops.tail)
+  }
+
+  @Test
+  def simpleGotoThrow(): Unit = {
+    val rest = List(
+      Op(ICONST_1),         // need some code, otherwise removeJumpToSuccessor kicks in
+      Op(POP),
+      Label(1),
+      Label(2),
+      Label(3),
+      Op(ATHROW)
+    )
+    val method = genMethod()(
+      Op(ACONST_NULL) ::
+      Jump(GOTO, Label(2)) :: // replaced by ATHROW
+      rest: _*
+    )
+    assertTrue(localOpt.simplifyJumps(method))
+    assertSameCode(instructionsFromMethod(method), Op(ACONST_NULL) :: Op(ATHROW) :: rest)
+  }
+
+  @Test
+  def gotoThrowInTry(): Unit = {
+    val handler = List(ExceptionHandler(Label(1), Label(2), Label(4), Some("java/lang/Throwable")))
+    val initialInstrs = List(
+      Label(1),
+      Op(ACONST_NULL),
+      Jump(GOTO, Label(3)), // not by ATHROW (would move the ATHROW into a try block)
+      Label(2),
+      Op(ICONST_1),         // need some code, otherwise removeJumpToSuccessor kicks in
+      Op(POP),
+      Label(3),
+      Op(ATHROW),
+      Label(4),
+      Op(POP),
+      Op(RETURN)
+    )
+    val method = genMethod(handlers = handler)(initialInstrs: _*)
+    assertFalse(localOpt.simplifyJumps(method))
+    assertSameCode(instructionsFromMethod(method), initialInstrs)
+
+    val optMethod = genMethod()(initialInstrs: _*) // no handler
+    assertTrue(localOpt.simplifyJumps(optMethod))
+    assertSameCode(instructionsFromMethod(optMethod).take(3), List(Label(1), Op(ACONST_NULL), Op(ATHROW)))
+  }
+
+  @Test
+  def simplifyBranchOverGoto(): Unit = {
+    val begin = List(
+      VarOp(ILOAD, 1),
+      Jump(IFGE, Label(2))
+    )
+    val rest = List(
+      Jump(GOTO, Label(3)),
+      Label(11), // other labels here are allowed
+      Label(2),
+      VarOp(ILOAD, 1),
+      Op(RETURN),
+      Label(3),
+      VarOp(ILOAD, 1),
+      Op(IRETURN)
+    )
+    val method = genMethod()(begin ::: rest: _*)
+    assertTrue(localOpt.simplifyJumps(method))
+    assertSameCode(
+      instructionsFromMethod(method),
+      List(VarOp(ILOAD, 1), Jump(IFLT, Label(3))) ::: rest.tail )
+
+    // 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))
+  }
+
+  @Test
+  def ensureGotoRemoved(): Unit = {
+    def code(jumps: Instruction*) = List(
+      VarOp(ILOAD, 1)) ::: jumps.toList ::: List(
+      Label(2),
+
+      Op(RETURN),
+      Label(3),
+      Op(RETURN)
+    )
+
+    // 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))
+    assertSameCode(instructionsFromMethod(method), code(Jump(IFLT, Label(3))))
+  }
+
+  @Test
+  def removeJumpToSuccessor(): Unit = {
+    val ops = List(
+      Jump(GOTO, Label(1)),
+      Label(11),
+      Label(1),
+      Label(2),
+      VarOp(ILOAD, 1),
+      Op(IRETURN)
+    )
+    val method = genMethod()(ops: _*)
+    assertTrue(localOpt.simplifyJumps(method))
+    assertSameCode(instructionsFromMethod(method), ops.tail)
+  }
+
+  @Test
+  def collapseJumpChains(): Unit = {
+    def ops(target1: Int, target2: Int, target3: Int) = List(
+      VarOp(ILOAD, 1),
+      Jump(IFGE, Label(target1)), // initially 1, then 3
+      VarOp(ILOAD, 1),
+      Op(IRETURN),
+
+      Label(2),
+      Jump(GOTO, Label(target3)),
+
+      Label(1),
+      Jump(GOTO, Label(target2)),     // initially 2, then 3
+
+      VarOp(ILOAD, 1),                // some code to prevent jumpToSuccessor optimization (once target2 is replaced by 3)
+      Op(RETURN),
+
+      Label(3),
+      VarOp(ILOAD, 1),
+      Op(IRETURN)
+    )
+    val method = genMethod()(ops(1, 2, 3): _*)
+    assertTrue(localOpt.simplifyJumps(method))
+    assertSameCode(instructionsFromMethod(method), ops(3, 3, 3))
+  }
+
+  @Test
+  def collapseJumpChainLoop(): Unit = {
+    def ops(target: Int) = List(
+      VarOp(ILOAD, 1),
+      Jump(IFGE, Label(target)),
+
+      Label(4),
+      Jump(GOTO, Label(3)),
+
+      VarOp(ILOAD, 1), // some code to prevent jumpToSuccessor (label 3)
+      Op(IRETURN),
+
+      Label(3),
+      Jump(GOTO, Label(4)),
+
+      Label(2),
+      Jump(GOTO, Label(3))
+    )
+
+    val method = genMethod()(ops(2): _*)
+    assertTrue(localOpt.simplifyJumps(method))
+    assertSameCode(instructionsFromMethod(method), ops(3))
+  }
+
+  @Test
+  def simplifyThenElseSameTarget(): Unit = {
+    def ops(jumpOp: Instruction) = List(
+      VarOp(ILOAD, 1),
+      jumpOp,
+      Label(2),
+      Jump(GOTO, Label(1)),
+
+      VarOp(ILOAD, 1), // some code to prevent jumpToSuccessor (label 1)
+      Op(IRETURN),
+
+      Label(1),
+      VarOp(ILOAD, 1),
+      Op(IRETURN)
+    )
+
+    val method = genMethod()(ops(Jump(IFGE, Label(1))): _*)
+    assertTrue(localOpt.simplifyJumps(method))
+    assertSameCode(instructionsFromMethod(method), ops(Op(POP)))
+  }
+
+  @Test
+  def thenElseSameTargetLoop(): Unit = {
+    def ops(br: List[Instruction]) = List(
+      VarOp(ILOAD, 1),
+      VarOp(ILOAD, 2)) ::: br ::: List(
+      Label(1),
+      Jump(GOTO, Label(1))
+    )
+    val method = genMethod()(ops(List(Jump(IF_ICMPGE, Label(1)))): _*)
+    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)
-  }
-}