From 257a7e65deb01c9e161c83ea5fd7a2b3c862e5e1 Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Mon, 4 Jul 2011 20:40:05 +0000 Subject: Fixed a bug in the optimizer which was preventi... Fixed a bug in the optimizer which was preventing private methods from being inlined. Also relaxes a condition related to the "liftedTry" problem: the inliner has to exclude certain methods from consideration if there is a value on the stack and the method being inlined has exception handlers. The new condition is as before, except that it does not exclude methods of the "try/finally" variety (i.e. finalizers, but no other exception handlers.) This is necessary to optimize this common pattern: @inline private def foo(body: => Unit) { val saved = something try body finally something = saved } The closure for "body" can be fully eliminated, but only if the contents of foo can be inlined into the caller. Closes #4764, review by rompf. --- test/files/pos/inliner2.flags | 1 + test/files/pos/inliner2.scala | 57 +++++++++++++++++++++++++++++++++++++ test/files/run/private-inline.check | 1 + test/files/run/private-inline.flags | 1 + test/files/run/private-inline.scala | 52 +++++++++++++++++++++++++++++++++ 5 files changed, 112 insertions(+) create mode 100644 test/files/pos/inliner2.flags create mode 100644 test/files/pos/inliner2.scala create mode 100644 test/files/run/private-inline.check create mode 100644 test/files/run/private-inline.flags create mode 100644 test/files/run/private-inline.scala (limited to 'test') diff --git a/test/files/pos/inliner2.flags b/test/files/pos/inliner2.flags new file mode 100644 index 0000000000..ea03113c66 --- /dev/null +++ b/test/files/pos/inliner2.flags @@ -0,0 +1 @@ +-optimise -Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/inliner2.scala b/test/files/pos/inliner2.scala new file mode 100644 index 0000000000..bc83e04312 --- /dev/null +++ b/test/files/pos/inliner2.scala @@ -0,0 +1,57 @@ +// This isn't actually testing much, because no warning is emitted in versions +// before the fix which comes with this because the method isn't even considered +// for inlining due to the bug. +class A { + private var debug = false + @inline private def ifelse[T](cond: => Boolean, ifPart: => T, elsePart: => T): T = + if (cond) ifPart else elsePart + + final def bob1() = ifelse(debug, 1, 2) + final def bob2() = if (debug) 1 else 2 +} +// Cool: +// +// % ls -1 /tmp/2901/ +// A$$anonfun$bob1$1.class +// A$$anonfun$bob1$2.class +// A$$anonfun$bob1$3.class +// A.class +// % ls -1 /tmp/trunk +// A.class +// +// Observations: +// +// (1) The inlined version accesses the field: the explicit one calls the accessor. +// (2) The inlined version fails to eliminate boxing. With reference types it emits +// an unneeded checkcast. +// (3) The private var debug is mangled to A$$debug, but after inlining it is never accessed +// from outside of the class and doesn't need mangling. +// (4) We could forego emitting bytecode for ifelse entirely if it has been +// inlined at all sites. +// +// Generated bytecode for the above: +// +// public final int bob1(); +// Code: +// Stack=1, Locals=1, Args_size=1 +// 0: aload_0 +// 1: getfield #11; //Field A$$debug:Z +// 4: ifeq 14 +// 7: iconst_1 +// 8: invokestatic #41; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer; +// 11: goto 18 +// 14: iconst_2 +// 15: invokestatic #41; //Method scala/runtime/BoxesRunTime.boxToInteger:(I)Ljava/lang/Integer; +// 18: invokestatic #45; //Method scala/runtime/BoxesRunTime.unboxToInt:(Ljava/lang/Object;)I +// 21: ireturn +// +// public final int bob2(); +// Code: +// Stack=1, Locals=1, Args_size=1 +// 0: aload_0 +// 1: invokevirtual #48; //Method A$$debug:()Z +// 4: ifeq 11 +// 7: iconst_1 +// 8: goto 12 +// 11: iconst_2 +// 12: ireturn diff --git a/test/files/run/private-inline.check b/test/files/run/private-inline.check new file mode 100644 index 0000000000..209e3ef4b6 --- /dev/null +++ b/test/files/run/private-inline.check @@ -0,0 +1 @@ +20 diff --git a/test/files/run/private-inline.flags b/test/files/run/private-inline.flags new file mode 100644 index 0000000000..eb4d19bcb9 --- /dev/null +++ b/test/files/run/private-inline.flags @@ -0,0 +1 @@ +-optimise \ No newline at end of file diff --git a/test/files/run/private-inline.scala b/test/files/run/private-inline.scala new file mode 100644 index 0000000000..5181ac8ba3 --- /dev/null +++ b/test/files/run/private-inline.scala @@ -0,0 +1,52 @@ + +final class A { + private var x1 = false + var x2 = false + + // manipulates private var + @inline private def wrapper1[T](body: => T): T = { + val saved = x1 + x1 = true + try body + finally x1 = saved + } + // manipulates public var + @inline private def wrapper2[T](body: => T): T = { + val saved = x2 + x2 = true + try body + finally x2 = saved + } + + // not inlined + def f1a() = wrapper1(5) + // inlined! + def f1b() = identity(wrapper1(5)) + + // not inlined + def f2a() = wrapper2(5) + // inlined! + def f2b() = identity(wrapper2(5)) +} + +object Test { + def methodClasses = List("f1a", "f1b", "f2a", "f2b") map ("A$$anonfun$" + _ + "$1") + + def main(args: Array[String]): Unit = { + val a = new A + import a._ + println(f1a() + f1b() + f2a() + f2b()) + + // Don't know how else to test this: all these should have been + // inlined, so all should fail. + methodClasses foreach { clazz => + + val foundClass = ( + try Class.forName(clazz) + catch { case _ => null } + ) + + assert(foundClass == null, foundClass) + } + } +} -- cgit v1.2.3