Rewrite copy propagation test

3 files changed, 73 insertions, 191 deletions

diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/MethodLevelOptsTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/MethodLevelOptsTest.scala
index 49b9924e97..7e5dd734fd 100644
--- a/test/junit/scala/tools/nsc/backend/jvm/opt/MethodLevelOptsTest.scala
+++ b/test/junit/scala/tools/nsc/backend/jvm/opt/MethodLevelOptsTest.scala
@@ -8,6 +8,7 @@ import org.junit.Test
 import scala.tools.asm.Opcodes._
 import org.junit.Assert._
 
+import scala.tools.asm.tree.ClassNode
 import scala.tools.nsc.backend.jvm.AsmUtils._
 import scala.tools.testing.AssertUtil._
 
@@ -30,6 +31,8 @@ class MethodLevelOptsTest extends ClearAfterClass {
 
   def wrapInDefault(code: Instruction*) = List(Label(0), LineNumber(1, Label(0))) ::: code.toList ::: List(Label(1))
 
+  def locals(c: ClassNode, m: String) = findAsmMethod(c, m).localVariables.asScala.toList.map(l => (l.name, l.index)).sortBy(_._2)
+
   @Test
   def eliminateEmptyTry(): Unit = {
     val code = "def f = { try {} catch { case _: Throwable => 0; () }; 1 }"
@@ -627,27 +630,91 @@ class MethodLevelOptsTest extends ClearAfterClass {
       """.stripMargin
 
     val List(c) = compileClasses(methodOptCompiler)(code)
-    def locals(m: String) = findAsmMethod(c, m).localVariables.asScala.toList.map(l => (l.name, l.index)).sortBy(_._2)
     def stores(m: String) = getSingleMethod(c, m).instructions.filter(_.opcode == ASTORE)
 
-    assertEquals(locals("t1"), List(("this",0), ("kept1",1), ("result",2)))
+    assertEquals(locals(c, "t1"), List(("this",0), ("kept1",1), ("result",2)))
     assert(stores("t1") == List(VarOp(ASTORE, 1), VarOp(ASTORE, 2), VarOp(ASTORE, 1), VarOp(ASTORE, 1)),
       textify(findAsmMethod(c, "t1")))
 
-    assertEquals(locals("t2"), List(("this",0), ("kept2",1), ("kept3",2)))
+    assertEquals(locals(c, "t2"), List(("this",0), ("kept2",1), ("kept3",2)))
     assert(stores("t2") == List(VarOp(ASTORE, 1), VarOp(ASTORE, 2), VarOp(ASTORE, 1)),
       textify(findAsmMethod(c, "t2")))
 
-    assertEquals(locals("t3"), List(("this",0), ("kept4",1)))
+    assertEquals(locals(c, "t3"), List(("this",0), ("kept4",1)))
     assert(stores("t3") == List(VarOp(ASTORE, 1), VarOp(ASTORE, 1)),
       textify(findAsmMethod(c, "t3")))
 
-    assertEquals(locals("t4"), List(("this",0), ("kept5",1)))
+    assertEquals(locals(c, "t4"), List(("this",0), ("kept5",1)))
     assert(stores("t4") == List(VarOp(ASTORE, 1), VarOp(ASTORE, 1)),
       textify(findAsmMethod(c, "t4")))
 
-    assertEquals(locals("t5"), List(("this",0), ("kept6",1)))
+    assertEquals(locals(c, "t5"), List(("this",0), ("kept6",1)))
     assert(stores("t5") == List(VarOp(ASTORE, 1), VarOp(ASTORE, 1)),
       textify(findAsmMethod(c, "t5")))
   }
+
+  @Test
+  def testCpp(): Unit = {
+    // copied from an old test (run/test-cpp.scala)
+    val code =
+      """class C {
+        |  import scala.util.Random._
+        |
+        |  def t1(x: Int) = {
+        |    val y = x
+        |    println(y)
+        |  }
+        |
+        |  def t2 = {
+        |    val x = 2
+        |    val y = x
+        |    println(y)
+        |  }
+        |
+        |  def t3 = {
+        |    val x = this
+        |    val y = x
+        |    println(y)
+        |  }
+        |
+        |  def f = nextInt
+        |
+        |  def t4 = {
+        |    val x = f
+        |    val y = x
+        |    println(y)
+        |  }
+        |
+        |  def t5 = {
+        |    var x = nextInt
+        |    var y = x
+        |    println(y)
+        |
+        |    y = nextInt
+        |    x = y
+        |    println(x)
+        |  }
+        |}
+      """.stripMargin
+
+    val List(c) = compileClasses(methodOptCompiler)(code)
+    assertEquals(locals(c, "t1"), List(("this", 0), ("x", 1)))
+
+    assertEquals(locals(c, "t2"), List(("this", 0), ("x", 1)))
+    // we don't have constant propagation (yet).
+    // the local var can't be optimized as a store;laod sequence, there's a GETSTATIC between the two
+    assertEquals(
+      textify(findAsmMethod(c, "t2")),
+      getSingleMethod(c, "t2").instructions.dropNonOp.map(_.opcode),
+      List(
+        ICONST_2, ISTORE,
+        GETSTATIC,           // Predef.MODULE$
+        ILOAD, INVOKESTATIC, // boxToInteger
+        INVOKEVIRTUAL,       // println
+        RETURN))
+
+    assertEquals(locals(c, "t3"), List(("this", 0)))
+    assertEquals(locals(c, "t4"), List(("this", 0), ("x", 1)))
+    assertEquals(locals(c, "t5"), List(("this", 0), ("x", 1)))
+  }
 }
diff --git a/test/pending/run/test-cpp.check b/test/pending/run/test-cpp.check
deleted file mode 100644
index ff4c9bf2bf..0000000000
--- a/test/pending/run/test-cpp.check
+++ /dev/null
@@ -1,81 +0,0 @@
---- a
-+++ b
-@@ -54,3 +54,3 @@
-   def main(args: Array[String] (ARRAY[REF(class String)])): Unit {
--  locals: value args, value x, value y
-+  locals: value args
-   startBlock: 1
-@@ -59,10 +59,6 @@
-   1:
--    52	CONSTANT(2)
--    52	STORE_LOCAL(value x)
-     52	SCOPE_ENTER value x
--    53	LOAD_LOCAL(value x)
--    53	STORE_LOCAL(value y)
-     53	SCOPE_ENTER value y
-     54	LOAD_MODULE object Predef
--    54	LOAD_LOCAL(value y)
-+    54	CONSTANT(2)
-     54	BOX INT
-@@ -109,3 +105,3 @@
-   def main(args: Array[String] (ARRAY[REF(class String)])): Unit {
--  locals: value args, value x, value y
-+  locals: value args, value x
-   startBlock: 1
-@@ -118,7 +114,5 @@
-     81	SCOPE_ENTER value x
--    82	LOAD_LOCAL(value x)
--    82	STORE_LOCAL(value y)
-     82	SCOPE_ENTER value y
-     83	LOAD_MODULE object Predef
--    83	LOAD_LOCAL(value y)
-+    83	LOAD_LOCAL(value x)
-     83	BOX INT
-@@ -152,3 +146,3 @@
-   def main(args: Array[String] (ARRAY[REF(class String)])): Unit {
--  locals: value args, value x, value y
-+  locals: value args
-   startBlock: 1
-@@ -157,10 +151,6 @@
-   1:
--    66	THIS(TestAliasChainDerefThis)
--    66	STORE_LOCAL(value x)
-     66	SCOPE_ENTER value x
--    67	LOAD_LOCAL(value x)
--    67	STORE_LOCAL(value y)
-     67	SCOPE_ENTER value y
-     68	LOAD_MODULE object Predef
--    68	LOAD_LOCAL(value y)
-+    68	THIS(Object)
-     68	CALL_METHOD scala.Predef.println (dynamic)
-@@ -193,3 +183,3 @@
-   def test(x: Int (INT)): Unit {
--  locals: value x, value y
-+  locals: value x
-   startBlock: 1
-@@ -198,7 +188,5 @@
-   1:
--    29	LOAD_LOCAL(value x)
--    29	STORE_LOCAL(value y)
-     29	SCOPE_ENTER value y
-     30	LOAD_MODULE object Predef
--    30	LOAD_LOCAL(value y)
-+    30	LOAD_LOCAL(value x)
-     30	BOX INT
-@@ -240,7 +228,5 @@
-     96	SCOPE_ENTER variable x
--    97	LOAD_LOCAL(variable x)
--    97	STORE_LOCAL(variable y)
-     97	SCOPE_ENTER variable y
-     98	LOAD_MODULE object Predef
--    98	LOAD_LOCAL(variable y)
-+    98	LOAD_LOCAL(variable x)
-     98	BOX INT
-@@ -250,6 +236,4 @@
-     100	STORE_LOCAL(variable y)
--    101	LOAD_LOCAL(variable y)
--    101	STORE_LOCAL(variable x)
-     102	LOAD_MODULE object Predef
--    102	LOAD_LOCAL(variable x)
-+    102	LOAD_LOCAL(variable y)
-     102	BOX INT
diff --git a/test/pending/run/test-cpp.scala b/test/pending/run/test-cpp.scala
deleted file mode 100644
index 4fca67d51e..0000000000
--- a/test/pending/run/test-cpp.scala
+++ /dev/null
@@ -1,104 +0,0 @@
-/**
- * The only change is in the decision to replace a LOAD_LOCAL(l)
- * in the copy-propagation performed before ClosureElimination.
- *
- * In the general case, the local variable 'l' is connected through
- * an alias chain with other local variables and at the end of the
- * alias chain there may be a Value, call it 'v'.
- *
- * If 'v' is cheaper to access (it is a Deref(This) or Const(_)), then
- * replace the instruction to load it from the cheaper place.
- * Otherwise, we use the local variable at the end of the alias chain
- * instead of 'l'.
- */
-
-import scala.tools.partest.IcodeComparison
-
-object Test extends IcodeComparison {
-  override def printIcodeAfterPhase = "dce"
-}
-
-import scala.util.Random._
-
-/**
- * The example in the bug report (Issue-5321): an alias chain which store
- * an Unknown. Should remove local variable 'y'.
- */
-object TestBugReport {
-  def test(x: Int) = {
-    val y = x
-    println(y)
-  }
-}
-
-/**
- * The code taken from scala.tools.nsc.settings.Settings:
- * After inlining of the setter is performed, there is an opportunity for
- * copy-propagation to eliminate some local variables.
- */
-object TestSetterInline {
-  private var _postSetHook: this.type => Unit = (x: this.type) => ()
-  def withPostSetHook(f: this.type => Unit): this.type = { _postSetHook = f ; this }
-}
-
-
-/**
- * The access of the local variable 'y' should be replaced by the
- * constant.
- */
-object TestAliasChainConstant {
-
-  def main(args: Array[String]): Unit = {
-    val x = 2
-    val y = x
-    println(y)
-  }
-}
-
-/**
- * At the end of the alias chain we have a reference to 'this'.
- * The local variables should be all discarded and replace by a
- * direct reference to this
- */
-class TestAliasChainDerefThis {
-
-  def main(args: Array[String]): Unit = {
-    val x = this
-    val y = x
-    println(y)
-  }
-}
-
-/**
- * At the end of the alias chain, there is the value of a field.
- * The use of variable 'y' should be replaced by 'x', not by an access
- * to the field 'f' since it is more costly.
- */
-object TestAliasChainDerefField {
-  def f = nextInt
-
-  def main(args: Array[String]): Unit = {
-    val x = f
-    val y = x
-    println(y)
-  }
-}
-
-
-/**
- * The first time 'println' is called, 'x' is replaced by 'y'
- * and the second time, 'y' is replaced by 'x'. But none of them
- * can be removed.
- */
-object TestDifferentBindings {
-
-  def main(args: Array[String]): Unit = {
-    var x = nextInt
-    var y = x
-    println(y)
-
-    y = nextInt
-    x = y
-    println(x)
-  }
-}