From 276c8f85af386ec95d8823dd421248c2aa90ed74 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Mon, 8 Jul 2019 22:24:38 +0200 Subject: [PATCH 1/7] Fix #502: Optimize `Array.apply([...])` to `[...]` --- compiler/src/dotty/tools/dotc/Compiler.scala | 1 + .../tools/dotc/transform/ArrayApply.scala | 53 +++++++++ .../tools/backend/jvm/ArrayApplyOptTest.scala | 101 ++++++++++++++++++ tests/run/i502.check | 2 + tests/run/i502.scala | 14 +++ tests/run/t6611b.scala | 6 ++ 6 files changed, 177 insertions(+) create mode 100644 compiler/src/dotty/tools/dotc/transform/ArrayApply.scala create mode 100644 compiler/test/dotty/tools/backend/jvm/ArrayApplyOptTest.scala create mode 100644 tests/run/i502.check create mode 100644 tests/run/i502.scala create mode 100644 tests/run/t6611b.scala diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index b087f313c33e..56e71c29110b 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -92,6 +92,7 @@ class Compiler { List(new Erasure) :: // Rewrite types to JVM model, erasing all type parameters, abstract types and refinements. List(new ElimErasedValueType, // Expand erased value types to their underlying implmementation types new VCElideAllocations, // Peep-hole optimization to eliminate unnecessary value class allocations + new ArrayApply, // Optimize `scala.Array.apply([....])` and `scala.Array.apply(..., [....])` into `[...]` new ElimPolyFunction, // Rewrite PolyFunction subclasses to FunctionN subclasses new TailRec, // Rewrite tail recursion to loops new Mixin, // Expand trait fields and trait initializers diff --git a/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala b/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala new file mode 100644 index 000000000000..75a9e476745a --- /dev/null +++ b/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala @@ -0,0 +1,53 @@ +package dotty.tools.dotc +package transform + +import core._ +import MegaPhase._ +import Contexts.Context +import Symbols._ +import Types._ +import StdNames._ +import ast.Trees._ +import dotty.tools.dotc.ast.tpd + + +/** This phase rewrites calls to `Array.apply` to primitive array instantion. + * + * Transforms `scala.Array.apply([....])` and `scala.Array.apply(..., [....])` into `[...]` + */ +class ArrayApply extends MiniPhase { + import tpd._ + + override def phaseName: String = "arrayApply" + + override def transformApply(tree: tpd.Apply)(implicit ctx: Context): tpd.Tree = { + if (tree.symbol.name == nme.apply && tree.symbol.owner == defn.ArrayModule) { // Is `Array.apply` + tree.args match { + case CleanTree(Apply(wrapRefArrayMeth, (seqLit: tpd.JavaSeqLiteral) :: Nil)) :: ct :: Nil + if defn.WrapArrayMethods().contains(wrapRefArrayMeth.symbol) && elideClassTag(ct) => + seqLit + + case elem0 :: CleanTree(Apply(wrapRefArrayMeth, (seqLit: tpd.JavaSeqLiteral) :: Nil)) :: Nil + if defn.WrapArrayMethods().contains(wrapRefArrayMeth.symbol) => + tpd.JavaSeqLiteral(elem0 :: seqLit.elems, seqLit.elemtpt) + + case _ => + tree + } + + } else tree + } + + // Only optimize when classtag is `ClassTag.apply` or `ClassTag.{Byte, Boolean, ...}` + private def elideClassTag(ct: Tree)(implicit ctx: Context): Boolean = { + ct.symbol.maybeOwner.companionModule == defn.ClassTagModule + } + + object CleanTree { + def unapply(tree: Tree)(implicit ctx: Context): Some[Tree] = tree match { + case Block(Nil, expr) => unapply(expr) + case Typed(expr, _) => unapply(expr) + case _ => Some(tree) + } + } +} diff --git a/compiler/test/dotty/tools/backend/jvm/ArrayApplyOptTest.scala b/compiler/test/dotty/tools/backend/jvm/ArrayApplyOptTest.scala new file mode 100644 index 000000000000..5be5a435a63b --- /dev/null +++ b/compiler/test/dotty/tools/backend/jvm/ArrayApplyOptTest.scala @@ -0,0 +1,101 @@ +package dotty.tools.backend.jvm + +import org.junit.Test +import org.junit.Assert._ + +import scala.tools.asm.Opcodes._ + +class ArrayApplyOptTest extends DottyBytecodeTest { + import ASMConverters._ + + @Test def testArrayEmptyGenericApply= { + test("Array[String]()", List(Op(ICONST_0), TypeOp(ANEWARRAY, "java/lang/String"), Op(POP), Op(RETURN))) + test("Array[Unit]()", List(Op(ICONST_0), TypeOp(ANEWARRAY, "scala/runtime/BoxedUnit"), Op(POP), Op(RETURN))) + test("Array[Object]()", List(Op(ICONST_0), TypeOp(ANEWARRAY, "java/lang/Object"), Op(POP), Op(RETURN))) + test("Array[Boolean]()", List(Op(ICONST_0), IntOp(NEWARRAY, 4), Op(POP), Op(RETURN))) + test("Array[Char]()", List(Op(ICONST_0), IntOp(NEWARRAY, 5), Op(POP), Op(RETURN))) + test("Array[Float]()", List(Op(ICONST_0), IntOp(NEWARRAY, 6), Op(POP), Op(RETURN))) + test("Array[Double]()", List(Op(ICONST_0), IntOp(NEWARRAY, 7), Op(POP), Op(RETURN))) + test("Array[Byte]()", List(Op(ICONST_0), IntOp(NEWARRAY, 8), Op(POP), Op(RETURN))) + test("Array[Short]()", List(Op(ICONST_0), IntOp(NEWARRAY, 9), Op(POP), Op(RETURN))) + test("Array[Int]()", List(Op(ICONST_0), IntOp(NEWARRAY, 10), Op(POP), Op(RETURN))) + test("Array[Long]()", List(Op(ICONST_0), IntOp(NEWARRAY, 11), Op(POP), Op(RETURN))) + test("Array[T]()", List(Op(ICONST_0), IntOp(NEWARRAY, 10), Op(POP), Op(RETURN))) + } + + @Test def testArrayGenericApply= { + test("""Array("a", "b")""", List(Op(ICONST_2), TypeOp(ANEWARRAY, "java/lang/String"), Op(DUP), Op(ICONST_0), Ldc(LDC, "a"), Op(AASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, "b"), Op(AASTORE), Op(POP), Op(RETURN))) + test("""Array[Object]("a", "b")""", List(Op(ICONST_2), TypeOp(ANEWARRAY, "java/lang/Object"), Op(DUP), Op(ICONST_0), Ldc(LDC, "a"), Op(AASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, "b"), Op(AASTORE), Op(POP), Op(RETURN))) + } + + @Test def testArrayApplyBoolean = + test("Array(true, false)", List(Op(ICONST_2), IntOp(NEWARRAY, 4), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(BASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_0), Op(BASTORE), Op(POP), Op(RETURN))) + + @Test def testArrayApplyByte = + test("Array[Byte](1, 2)", List(Op(ICONST_2), IntOp(NEWARRAY, 8), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(BASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(BASTORE), Op(POP), Op(RETURN))) + + @Test def testArrayApplyShort = + test("Array[Short](1, 2)", List(Op(ICONST_2), IntOp(NEWARRAY, 9), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(SASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(SASTORE), Op(POP), Op(RETURN))) + + @Test def testArrayApplyInt = { + test("Array(1, 2)", List(Op(ICONST_2), IntOp(NEWARRAY, 10), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(IASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(IASTORE), Op(POP), Op(RETURN))) + test("""Array[T](t, t)""", List(Op(ICONST_2), IntOp(NEWARRAY, 10), Op(DUP), Op(ICONST_0), Field(GETSTATIC, "Foo$", "MODULE$", "LFoo$;"), Invoke(INVOKEVIRTUAL, "Foo$", "t", "()I", false), Op(IASTORE), Op(DUP), Op(ICONST_1), Field(GETSTATIC, "Foo$", "MODULE$", "LFoo$;"), Invoke(INVOKEVIRTUAL, "Foo$", "t", "()I", false), Op(IASTORE), Op(POP), Op(RETURN))) + } + + @Test def testArrayApplyLong = + test("Array(2L, 3L)", List(Op(ICONST_2), IntOp(NEWARRAY, 11), Op(DUP), Op(ICONST_0), Ldc(LDC, 2), Op(LASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, 3), Op(LASTORE), Op(POP), Op(RETURN))) + + @Test def testArrayApplyFloat = + test("Array(2.1f, 3.1f)", List(Op(ICONST_2), IntOp(NEWARRAY, 6), Op(DUP), Op(ICONST_0), Ldc(LDC, 2.1f), Op(FASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, 3.1f), Op(FASTORE), Op(POP), Op(RETURN))) + + @Test def testArrayApplyDouble = + test("Array(2.2d, 3.2d)", List(Op(ICONST_2), IntOp(NEWARRAY, 7), Op(DUP), Op(ICONST_0), Ldc(LDC, 2.2d), Op(DASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, 3.2d), Op(DASTORE), Op(POP), Op(RETURN))) + + @Test def testArrayApplyChar = + test("Array('x', 'y')", List(Op(ICONST_2), IntOp(NEWARRAY, 5), Op(DUP), Op(ICONST_0), IntOp(BIPUSH, 120), Op(CASTORE), Op(DUP), Op(ICONST_1), IntOp(BIPUSH, 121), Op(CASTORE), Op(POP), Op(RETURN))) + + @Test def testArrayApplyUnit = + test("Array[Unit]((), ())", List(Op(ICONST_2), TypeOp(ANEWARRAY, "scala/runtime/BoxedUnit"), Op(DUP), + Op(ICONST_0), Field(GETSTATIC, "scala/runtime/BoxedUnit", "UNIT", "Lscala/runtime/BoxedUnit;"), Op(AASTORE), Op(DUP), + Op(ICONST_1), Field(GETSTATIC, "scala/runtime/BoxedUnit", "UNIT", "Lscala/runtime/BoxedUnit;"), Op(AASTORE), Op(POP), Op(RETURN))) + + @Test def testArrayInlined = test( + """{ + | inline def array(xs: =>Int*): Array[Int] = Array(xs: _*) + | array(1, 2) + |}""".stripMargin, + List(Op(ICONST_2), IntOp(NEWARRAY, 10), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(IASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(IASTORE), TypeOp(CHECKCAST, "[I"), Op(POP), Op(RETURN)) + ) + + @Test def testArrayInlined2 = test( + """{ + | inline def array(x: =>Int, xs: =>Int*): Array[Int] = Array(x, xs: _*) + | array(1, 2) + |}""".stripMargin, + List(Op(ICONST_2), IntOp(NEWARRAY, 10), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(IASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(IASTORE), Op(POP), Op(RETURN)) + ) + + private def test(code: String, expectedInstructions: List[Any])= { + val source = + s"""class Foo { + | import Foo._ + | def test: Unit = $code + |} + |object Foo { + | opaque type T = Int + | def t: T = 1 + |} + """.stripMargin + + checkBCode(source) { dir => + val clsIn = dir.lookupName("Foo.class", directory = false).input + val clsNode = loadClassNode(clsIn) + val meth = getMethod(clsNode, "test") + + val instructions = instructionsFromMethod(meth) + + assertEquals(expectedInstructions, instructions) + } + } + +} diff --git a/tests/run/i502.check b/tests/run/i502.check new file mode 100644 index 000000000000..e7e102616928 --- /dev/null +++ b/tests/run/i502.check @@ -0,0 +1,2 @@ +Ok +foo diff --git a/tests/run/i502.scala b/tests/run/i502.scala new file mode 100644 index 000000000000..27c3dffa5a4d --- /dev/null +++ b/tests/run/i502.scala @@ -0,0 +1,14 @@ +import scala.reflect.ClassTag + +object Test extends App { + Array[Int](1, 2) + + try { + Array[Int](1, 2)(null) + ??? + } catch { + case _: NullPointerException => println("Ok") + } + + Array[Int](1, 2)({println("foo"); the[ClassTag[Int]]}) +} diff --git a/tests/run/t6611b.scala b/tests/run/t6611b.scala new file mode 100644 index 000000000000..1c3cdf8d9742 --- /dev/null +++ b/tests/run/t6611b.scala @@ -0,0 +1,6 @@ +object Test extends App { + val a = Array("1") + val a2 = Array(a: _*) + a2(0) = "2" + assert(a(0) == "1") +} From 772f5b9771d787e6e9dd3275d8f0852cd776f035 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Tue, 9 Jul 2019 19:36:20 +0200 Subject: [PATCH 2/7] Factor out test code --- .../tools/backend/jvm/ArrayApplyOptTest.scala | 52 +++++++++++-------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/compiler/test/dotty/tools/backend/jvm/ArrayApplyOptTest.scala b/compiler/test/dotty/tools/backend/jvm/ArrayApplyOptTest.scala index 5be5a435a63b..1089abf331a8 100644 --- a/compiler/test/dotty/tools/backend/jvm/ArrayApplyOptTest.scala +++ b/compiler/test/dotty/tools/backend/jvm/ArrayApplyOptTest.scala @@ -12,47 +12,49 @@ class ArrayApplyOptTest extends DottyBytecodeTest { test("Array[String]()", List(Op(ICONST_0), TypeOp(ANEWARRAY, "java/lang/String"), Op(POP), Op(RETURN))) test("Array[Unit]()", List(Op(ICONST_0), TypeOp(ANEWARRAY, "scala/runtime/BoxedUnit"), Op(POP), Op(RETURN))) test("Array[Object]()", List(Op(ICONST_0), TypeOp(ANEWARRAY, "java/lang/Object"), Op(POP), Op(RETURN))) - test("Array[Boolean]()", List(Op(ICONST_0), IntOp(NEWARRAY, 4), Op(POP), Op(RETURN))) - test("Array[Char]()", List(Op(ICONST_0), IntOp(NEWARRAY, 5), Op(POP), Op(RETURN))) - test("Array[Float]()", List(Op(ICONST_0), IntOp(NEWARRAY, 6), Op(POP), Op(RETURN))) - test("Array[Double]()", List(Op(ICONST_0), IntOp(NEWARRAY, 7), Op(POP), Op(RETURN))) - test("Array[Byte]()", List(Op(ICONST_0), IntOp(NEWARRAY, 8), Op(POP), Op(RETURN))) - test("Array[Short]()", List(Op(ICONST_0), IntOp(NEWARRAY, 9), Op(POP), Op(RETURN))) - test("Array[Int]()", List(Op(ICONST_0), IntOp(NEWARRAY, 10), Op(POP), Op(RETURN))) - test("Array[Long]()", List(Op(ICONST_0), IntOp(NEWARRAY, 11), Op(POP), Op(RETURN))) - test("Array[T]()", List(Op(ICONST_0), IntOp(NEWARRAY, 10), Op(POP), Op(RETURN))) + test("Array[Boolean]()", newArray0Opcodes(T_BOOLEAN)) + test("Array[Byte]()", newArray0Opcodes(T_BYTE)) + test("Array[Short]()", newArray0Opcodes(T_SHORT)) + test("Array[Int]()", newArray0Opcodes(T_INT)) + test("Array[Long]()", newArray0Opcodes(T_LONG)) + test("Array[Float]()", newArray0Opcodes(T_FLOAT)) + test("Array[Double]()", newArray0Opcodes(T_DOUBLE)) + test("Array[Char]()", newArray0Opcodes(T_CHAR)) + test("Array[T]()", newArray0Opcodes(T_INT)) } @Test def testArrayGenericApply= { - test("""Array("a", "b")""", List(Op(ICONST_2), TypeOp(ANEWARRAY, "java/lang/String"), Op(DUP), Op(ICONST_0), Ldc(LDC, "a"), Op(AASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, "b"), Op(AASTORE), Op(POP), Op(RETURN))) - test("""Array[Object]("a", "b")""", List(Op(ICONST_2), TypeOp(ANEWARRAY, "java/lang/Object"), Op(DUP), Op(ICONST_0), Ldc(LDC, "a"), Op(AASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, "b"), Op(AASTORE), Op(POP), Op(RETURN))) + def opCodes(tpe: String) = + List(Op(ICONST_2), TypeOp(ANEWARRAY, tpe), Op(DUP), Op(ICONST_0), Ldc(LDC, "a"), Op(AASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, "b"), Op(AASTORE), Op(POP), Op(RETURN)) + test("""Array("a", "b")""", opCodes("java/lang/String")) + test("""Array[Object]("a", "b")""", opCodes("java/lang/Object")) } @Test def testArrayApplyBoolean = - test("Array(true, false)", List(Op(ICONST_2), IntOp(NEWARRAY, 4), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(BASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_0), Op(BASTORE), Op(POP), Op(RETURN))) + test("Array(true, false)", newArray2Opcodes(T_BOOLEAN, List(Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(BASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_0), Op(BASTORE)))) @Test def testArrayApplyByte = - test("Array[Byte](1, 2)", List(Op(ICONST_2), IntOp(NEWARRAY, 8), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(BASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(BASTORE), Op(POP), Op(RETURN))) + test("Array[Byte](1, 2)", newArray2Opcodes(T_BYTE, List(Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(BASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(BASTORE)))) @Test def testArrayApplyShort = - test("Array[Short](1, 2)", List(Op(ICONST_2), IntOp(NEWARRAY, 9), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(SASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(SASTORE), Op(POP), Op(RETURN))) + test("Array[Short](1, 2)", newArray2Opcodes(T_SHORT, List(Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(SASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(SASTORE)))) @Test def testArrayApplyInt = { - test("Array(1, 2)", List(Op(ICONST_2), IntOp(NEWARRAY, 10), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(IASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(IASTORE), Op(POP), Op(RETURN))) - test("""Array[T](t, t)""", List(Op(ICONST_2), IntOp(NEWARRAY, 10), Op(DUP), Op(ICONST_0), Field(GETSTATIC, "Foo$", "MODULE$", "LFoo$;"), Invoke(INVOKEVIRTUAL, "Foo$", "t", "()I", false), Op(IASTORE), Op(DUP), Op(ICONST_1), Field(GETSTATIC, "Foo$", "MODULE$", "LFoo$;"), Invoke(INVOKEVIRTUAL, "Foo$", "t", "()I", false), Op(IASTORE), Op(POP), Op(RETURN))) + test("Array(1, 2)", newArray2Opcodes(T_INT, List(Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(IASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(IASTORE)))) + test("""Array[T](t, t)""", newArray2Opcodes(T_INT, List(Op(DUP), Op(ICONST_0), Field(GETSTATIC, "Foo$", "MODULE$", "LFoo$;"), Invoke(INVOKEVIRTUAL, "Foo$", "t", "()I", false), Op(IASTORE), Op(DUP), Op(ICONST_1), Field(GETSTATIC, "Foo$", "MODULE$", "LFoo$;"), Invoke(INVOKEVIRTUAL, "Foo$", "t", "()I", false), Op(IASTORE)))) } @Test def testArrayApplyLong = - test("Array(2L, 3L)", List(Op(ICONST_2), IntOp(NEWARRAY, 11), Op(DUP), Op(ICONST_0), Ldc(LDC, 2), Op(LASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, 3), Op(LASTORE), Op(POP), Op(RETURN))) + test("Array(2L, 3L)", newArray2Opcodes(T_LONG, List(Op(DUP), Op(ICONST_0), Ldc(LDC, 2), Op(LASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, 3), Op(LASTORE)))) @Test def testArrayApplyFloat = - test("Array(2.1f, 3.1f)", List(Op(ICONST_2), IntOp(NEWARRAY, 6), Op(DUP), Op(ICONST_0), Ldc(LDC, 2.1f), Op(FASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, 3.1f), Op(FASTORE), Op(POP), Op(RETURN))) + test("Array(2.1f, 3.1f)", newArray2Opcodes(T_FLOAT, List(Op(DUP), Op(ICONST_0), Ldc(LDC, 2.1f), Op(FASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, 3.1f), Op(FASTORE)))) @Test def testArrayApplyDouble = - test("Array(2.2d, 3.2d)", List(Op(ICONST_2), IntOp(NEWARRAY, 7), Op(DUP), Op(ICONST_0), Ldc(LDC, 2.2d), Op(DASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, 3.2d), Op(DASTORE), Op(POP), Op(RETURN))) + test("Array(2.2d, 3.2d)", newArray2Opcodes(T_DOUBLE, List(Op(DUP), Op(ICONST_0), Ldc(LDC, 2.2d), Op(DASTORE), Op(DUP), Op(ICONST_1), Ldc(LDC, 3.2d), Op(DASTORE)))) @Test def testArrayApplyChar = - test("Array('x', 'y')", List(Op(ICONST_2), IntOp(NEWARRAY, 5), Op(DUP), Op(ICONST_0), IntOp(BIPUSH, 120), Op(CASTORE), Op(DUP), Op(ICONST_1), IntOp(BIPUSH, 121), Op(CASTORE), Op(POP), Op(RETURN))) + test("Array('x', 'y')", newArray2Opcodes(T_CHAR, List(Op(DUP), Op(ICONST_0), IntOp(BIPUSH, 120), Op(CASTORE), Op(DUP), Op(ICONST_1), IntOp(BIPUSH, 121), Op(CASTORE)))) @Test def testArrayApplyUnit = test("Array[Unit]((), ())", List(Op(ICONST_2), TypeOp(ANEWARRAY, "scala/runtime/BoxedUnit"), Op(DUP), @@ -64,7 +66,7 @@ class ArrayApplyOptTest extends DottyBytecodeTest { | inline def array(xs: =>Int*): Array[Int] = Array(xs: _*) | array(1, 2) |}""".stripMargin, - List(Op(ICONST_2), IntOp(NEWARRAY, 10), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(IASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(IASTORE), TypeOp(CHECKCAST, "[I"), Op(POP), Op(RETURN)) + newArray2Opcodes(T_INT, List(Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(IASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(IASTORE), TypeOp(CHECKCAST, "[I"))) ) @Test def testArrayInlined2 = test( @@ -72,9 +74,15 @@ class ArrayApplyOptTest extends DottyBytecodeTest { | inline def array(x: =>Int, xs: =>Int*): Array[Int] = Array(x, xs: _*) | array(1, 2) |}""".stripMargin, - List(Op(ICONST_2), IntOp(NEWARRAY, 10), Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(IASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(IASTORE), Op(POP), Op(RETURN)) + newArray2Opcodes(T_INT, List(Op(DUP), Op(ICONST_0), Op(ICONST_1), Op(IASTORE), Op(DUP), Op(ICONST_1), Op(ICONST_2), Op(IASTORE))) ) + private def newArray0Opcodes(tpe: Int, init: List[Any] = Nil): List[Any] = + Op(ICONST_0) :: IntOp(NEWARRAY, tpe) :: init ::: Op(POP) :: Op(RETURN) :: Nil + + private def newArray2Opcodes(tpe: Int, init: List[Any] = Nil): List[Any] = + Op(ICONST_2) :: IntOp(NEWARRAY, tpe) :: init ::: Op(POP) :: Op(RETURN) :: Nil + private def test(code: String, expectedInstructions: List[Any])= { val source = s"""class Foo { From f3beaa8d93da7d57e857f1882d8e4497803a453f Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Tue, 9 Jul 2019 20:30:18 +0200 Subject: [PATCH 3/7] Optimize Array.apply only when ClassTag is statically known --- .../dotty/tools/dotc/core/Definitions.scala | 2 ++ .../tools/dotc/transform/ArrayApply.scala | 26 ++++++++++++++++--- tests/run/i502.check | 1 + tests/run/i502.scala | 2 ++ 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 31804f36b9cb..c76c3cf75da6 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -755,6 +755,8 @@ class Definitions { @threadUnsafe lazy val ClassTagType: TypeRef = ctx.requiredClassRef("scala.reflect.ClassTag") def ClassTagClass(implicit ctx: Context): ClassSymbol = ClassTagType.symbol.asClass def ClassTagModule(implicit ctx: Context): Symbol = ClassTagClass.companionModule + @threadUnsafe lazy val ClassTagModule_applyR: TermRef = ClassTagModule.requiredMethodRef(nme.apply) + def ClassTagModule_apply(implicit ctx: Context): Symbol = ClassTagModule_applyR.symbol @threadUnsafe lazy val QuotedExprType: TypeRef = ctx.requiredClassRef("scala.quoted.Expr") def QuotedExprClass(implicit ctx: Context): ClassSymbol = QuotedExprType.symbol.asClass diff --git a/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala b/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala index 75a9e476745a..d9b5cffe1d97 100644 --- a/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala +++ b/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala @@ -10,6 +10,8 @@ import StdNames._ import ast.Trees._ import dotty.tools.dotc.ast.tpd +import scala.reflect.ClassTag + /** This phase rewrites calls to `Array.apply` to primitive array instantion. * @@ -38,9 +40,27 @@ class ArrayApply extends MiniPhase { } else tree } - // Only optimize when classtag is `ClassTag.apply` or `ClassTag.{Byte, Boolean, ...}` - private def elideClassTag(ct: Tree)(implicit ctx: Context): Boolean = { - ct.symbol.maybeOwner.companionModule == defn.ClassTagModule + /** Only optimize when classtag if it is one of + * - `ClassTag.apply(classOf[X])` + * - `ClassTag.apply(java.lang.XYZ.Type)` + * - `ClassTag.{Byte, Boolean, ...}` + */ + private def elideClassTag(ct: Tree)(implicit ctx: Context): Boolean = ct match { + case Apply(_, rc :: Nil) if ct.symbol == defn.ClassTagModule_apply => + rc match { + case _: Literal => true // classOf[X] + case rc: RefTree if rc.name == nme.TYPE_ => + val owner = rc.symbol.maybeOwner.companionModule + owner == defn.BoxedBooleanModule || owner == defn.BoxedByteModule || + owner == defn.BoxedShortModule || owner == defn.BoxedCharModule || + owner == defn.BoxedIntModule || owner == defn.BoxedLongModule || + owner == defn.BoxedFloatModule || owner == defn.BoxedDoubleModule || + owner == defn.BoxedUnitModule + case _ => false + } + case Apply(ctm: RefTree, _) if ctm.symbol.maybeOwner.companionModule == defn.ClassTagModule => + nme.ScalaValueNames.contains(ctm.name) + case _ => false } object CleanTree { diff --git a/tests/run/i502.check b/tests/run/i502.check index e7e102616928..1bd58020669f 100644 --- a/tests/run/i502.check +++ b/tests/run/i502.check @@ -1,2 +1,3 @@ Ok foo +bar diff --git a/tests/run/i502.scala b/tests/run/i502.scala index 27c3dffa5a4d..c56825c134b0 100644 --- a/tests/run/i502.scala +++ b/tests/run/i502.scala @@ -11,4 +11,6 @@ object Test extends App { } Array[Int](1, 2)({println("foo"); the[ClassTag[Int]]}) + + Array[Int](1, 2)(ClassTag.apply({ println("bar"); classOf[Int]})) } From 73a8c8356cfa82e26ceeebfc57a723e970b39e12 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Tue, 9 Jul 2019 20:38:30 +0200 Subject: [PATCH 4/7] Only strip the type ascription --- compiler/src/dotty/tools/dotc/transform/ArrayApply.scala | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala b/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala index d9b5cffe1d97..94793b33f05a 100644 --- a/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala +++ b/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala @@ -25,11 +25,11 @@ class ArrayApply extends MiniPhase { override def transformApply(tree: tpd.Apply)(implicit ctx: Context): tpd.Tree = { if (tree.symbol.name == nme.apply && tree.symbol.owner == defn.ArrayModule) { // Is `Array.apply` tree.args match { - case CleanTree(Apply(wrapRefArrayMeth, (seqLit: tpd.JavaSeqLiteral) :: Nil)) :: ct :: Nil + case StripAscription(Apply(wrapRefArrayMeth, (seqLit: tpd.JavaSeqLiteral) :: Nil)) :: ct :: Nil if defn.WrapArrayMethods().contains(wrapRefArrayMeth.symbol) && elideClassTag(ct) => seqLit - case elem0 :: CleanTree(Apply(wrapRefArrayMeth, (seqLit: tpd.JavaSeqLiteral) :: Nil)) :: Nil + case elem0 :: StripAscription(Apply(wrapRefArrayMeth, (seqLit: tpd.JavaSeqLiteral) :: Nil)) :: Nil if defn.WrapArrayMethods().contains(wrapRefArrayMeth.symbol) => tpd.JavaSeqLiteral(elem0 :: seqLit.elems, seqLit.elemtpt) @@ -63,9 +63,8 @@ class ArrayApply extends MiniPhase { case _ => false } - object CleanTree { + object StripAscription { def unapply(tree: Tree)(implicit ctx: Context): Some[Tree] = tree match { - case Block(Nil, expr) => unapply(expr) case Typed(expr, _) => unapply(expr) case _ => Some(tree) } From 7db80833cd4e8612b784f9951aad7deb9d881fd5 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Tue, 9 Jul 2019 20:40:47 +0200 Subject: [PATCH 5/7] Add some documentation --- .../src/dotty/tools/dotc/transform/ArrayApply.scala | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala b/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala index 94793b33f05a..d921190d01d9 100644 --- a/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala +++ b/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala @@ -41,15 +41,15 @@ class ArrayApply extends MiniPhase { } /** Only optimize when classtag if it is one of - * - `ClassTag.apply(classOf[X])` - * - `ClassTag.apply(java.lang.XYZ.Type)` - * - `ClassTag.{Byte, Boolean, ...}` + * - `ClassTag.apply(classOf[XYZ])` + * - `ClassTag.apply(java.lang.XYZ.Type)` for boxed primitives `XYZ`` + * - `ClassTag.XYZ` for primitive types */ private def elideClassTag(ct: Tree)(implicit ctx: Context): Boolean = ct match { case Apply(_, rc :: Nil) if ct.symbol == defn.ClassTagModule_apply => rc match { - case _: Literal => true // classOf[X] - case rc: RefTree if rc.name == nme.TYPE_ => + case _: Literal => true // ClassTag.apply(classOf[XYZ]) + case rc: RefTree if rc.name == nme.TYPE_ => // ClassTag.apply(java.lang.XYZ.Type) val owner = rc.symbol.maybeOwner.companionModule owner == defn.BoxedBooleanModule || owner == defn.BoxedByteModule || owner == defn.BoxedShortModule || owner == defn.BoxedCharModule || @@ -59,6 +59,7 @@ class ArrayApply extends MiniPhase { case _ => false } case Apply(ctm: RefTree, _) if ctm.symbol.maybeOwner.companionModule == defn.ClassTagModule => + // ClassTag.XYZ nme.ScalaValueNames.contains(ctm.name) case _ => false } From 4cdbcdaba5314dd60579bde094d7d75da60cf3b2 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Wed, 10 Jul 2019 07:26:48 +0200 Subject: [PATCH 6/7] Use ScalaBoxedClasses --- .../src/dotty/tools/dotc/transform/ArrayApply.scala | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala b/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala index d921190d01d9..65a77c89fd57 100644 --- a/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala +++ b/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala @@ -49,13 +49,9 @@ class ArrayApply extends MiniPhase { case Apply(_, rc :: Nil) if ct.symbol == defn.ClassTagModule_apply => rc match { case _: Literal => true // ClassTag.apply(classOf[XYZ]) - case rc: RefTree if rc.name == nme.TYPE_ => // ClassTag.apply(java.lang.XYZ.Type) - val owner = rc.symbol.maybeOwner.companionModule - owner == defn.BoxedBooleanModule || owner == defn.BoxedByteModule || - owner == defn.BoxedShortModule || owner == defn.BoxedCharModule || - owner == defn.BoxedIntModule || owner == defn.BoxedLongModule || - owner == defn.BoxedFloatModule || owner == defn.BoxedDoubleModule || - owner == defn.BoxedUnitModule + case rc: RefTree if rc.name == nme.TYPE_ => + // ClassTag.apply(java.lang.XYZ.Type) + defn.ScalaBoxedClasses().contains(rc.symbol.maybeOwner.companionClass) case _ => false } case Apply(ctm: RefTree, _) if ctm.symbol.maybeOwner.companionModule == defn.ClassTagModule => From 154181716dddd546e4f628aacec7351ba02537ce Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Wed, 10 Jul 2019 10:39:44 +0200 Subject: [PATCH 7/7] Fix documentation --- compiler/src/dotty/tools/dotc/transform/ArrayApply.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala b/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala index 65a77c89fd57..39260a98850b 100644 --- a/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala +++ b/compiler/src/dotty/tools/dotc/transform/ArrayApply.scala @@ -13,7 +13,7 @@ import dotty.tools.dotc.ast.tpd import scala.reflect.ClassTag -/** This phase rewrites calls to `Array.apply` to primitive array instantion. +/** This phase rewrites calls to `Array.apply` to a direct instantiation of the array in the bytecode. * * Transforms `scala.Array.apply([....])` and `scala.Array.apply(..., [....])` into `[...]` */