From c35c9c77a976b06aeae04d15493ec4995a2a6448 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Thu, 27 Aug 2015 22:12:28 +0200 Subject: Revert workaround for SI-8334 The new optimizer doesn't have this problem. --- test/files/pos/list-optim-check.flags | 1 - test/files/pos/list-optim-check.scala | 21 --------------------- test/files/run/t8334.scala | 17 +++++++++++++++++ 3 files changed, 17 insertions(+), 22 deletions(-) delete mode 100644 test/files/pos/list-optim-check.flags delete mode 100644 test/files/pos/list-optim-check.scala create mode 100644 test/files/run/t8334.scala (limited to 'test/files') diff --git a/test/files/pos/list-optim-check.flags b/test/files/pos/list-optim-check.flags deleted file mode 100644 index 49d036a887..0000000000 --- a/test/files/pos/list-optim-check.flags +++ /dev/null @@ -1 +0,0 @@ --optimize diff --git a/test/files/pos/list-optim-check.scala b/test/files/pos/list-optim-check.scala deleted file mode 100644 index f6e6ddec77..0000000000 --- a/test/files/pos/list-optim-check.scala +++ /dev/null @@ -1,21 +0,0 @@ -// Tests a map known to crash in optimizer with faster List map in SI-8240. -// Equivalent tests for collect and flatmap do not crash, but are provided -// anyway. -// See ticket SI-8334 for optimizer bug. -// TODO - Remove this test once SI-8334 is fixed and has its own test. -class A { - def f: Boolean = { - val xs = Nil map (_ => return false) - true - } - - def g: Boolean = { - val xs = Nil collect { case _ => return false } - true - } - - def h: Boolean = { - val xs = Nil flatMap { _ => return false } - true - } -} diff --git a/test/files/run/t8334.scala b/test/files/run/t8334.scala new file mode 100644 index 0000000000..bc7e97bd04 --- /dev/null +++ b/test/files/run/t8334.scala @@ -0,0 +1,17 @@ +object Test extends App { + def f: Boolean = { + val xs = Nil map (_ => return false) + true + } + + def g: Boolean = { + val xs = Nil collect { case _ => return false } + true + } + + def h: Boolean = { + val xs = Nil flatMap { _ => return false } + true + } + assert(f && g && h) +} -- cgit v1.2.3 From 59a5d3b8fd036d36afcf6349bc3cc527344981a1 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Fri, 18 Sep 2015 10:26:01 +0200 Subject: Emit exception handlers for inlined methods in the correct order Handler tables are lists of tuples (try-start, try-end, handler-start, exception-type). When an instruction throws, the first handler in the list that covers the instruction and matches the type is executed. For nested handlers, it is the job of the compiler to add them to the handler table in the correct order. When inlining a method, the handlers of the callee are prepended to the list of handlers in the callsite method. This ensures that the callee's handlers are tested first if an exception is thrown in the inlined code. --- src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala | 6 +++++- test/files/run/inlineHandlers.scala | 7 +++++++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 test/files/run/inlineHandlers.scala (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala index 54f1b99876..20256ca63b 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala @@ -409,7 +409,11 @@ class Inliner[BT <: BTypes](val btypes: BT) { callsiteMethod.instructions.remove(callsiteInstruction) callsiteMethod.localVariables.addAll(cloneLocalVariableNodes(callee, labelsMap, callee.name + "_", localVarShift).asJava) - callsiteMethod.tryCatchBlocks.addAll(cloneTryCatchBlockNodes(callee, labelsMap).asJava) + // prepend the handlers of the callee. the order of handlers matters: when an exception is thrown + // at some instruction, the first handler guarding that instruction and having a matching exception + // type is executed. prepending the callee's handlers makes sure to test those handlers first if + // an exception is thrown in the inlined code. + callsiteMethod.tryCatchBlocks.addAll(0, cloneTryCatchBlockNodes(callee, labelsMap).asJava) callsiteMethod.maxLocals += returnType.getSize + callee.maxLocals val maxStackOfInlinedCode = { diff --git a/test/files/run/inlineHandlers.scala b/test/files/run/inlineHandlers.scala new file mode 100644 index 0000000000..8c672a07b9 --- /dev/null +++ b/test/files/run/inlineHandlers.scala @@ -0,0 +1,7 @@ +object Test { + @noinline def ham: String = throw null + @inline def inner: String = try { ham } catch { case _: NullPointerException => "npe" } + def foo = try inner catch { case e: Throwable => throw e } + + def main(args: Array[String]): Unit = assert(foo == "npe") +} -- cgit v1.2.3 From 41e5ef4396a1526cd8c58157912ffed830a96f1e Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Thu, 17 Sep 2015 20:15:36 +0200 Subject: First version of inliningh heuristics that prefer higher-order methos When invoking a higher-order method and the value passed for the SAM type is either a function literal or a parameter of the callsite method, inline the higher-order method into the callee. This is a first version, the heuristics will be refined further. --- .../nsc/backend/jvm/opt/InlinerHeuristics.scala | 12 ++++++-- .../scala/tools/nsc/settings/ScalaSettings.scala | 4 +-- test/files/run/mapConserve.scala | 2 +- .../tools/nsc/backend/jvm/opt/InlinerTest.scala | 33 ++++++++++++++++++++++ 4 files changed, 46 insertions(+), 5 deletions(-) (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala index cd10094204..e559b63c09 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/InlinerHeuristics.scala @@ -84,8 +84,16 @@ class InlinerHeuristics[BT <: BTypes](val bTypes: BT) { if (callee.safeToInline && callee.annotatedInline) Some(InlineRequest(callsite, Nil)) else None -// case "default" => - + case "default" => + val callee = callsite.callee.get + if (callee.safeToInline && !callee.annotatedNoInline) { + val shouldInlineHO = callee.samParamTypes.nonEmpty && (callee.samParamTypes exists { + case (index, _) => callsite.argInfos.contains(index) + }) + + if (shouldInlineHO || callee.annotatedInline) Some(InlineRequest(callsite, Nil)) + else None + } else None } /* diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala index ea9ad1d909..74d152a4cf 100644 --- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala +++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala @@ -276,8 +276,8 @@ trait ScalaSettings extends AbsScalaSettings name = "-Yopt-inline-heuristics", helpArg = "strategy", descr = "Set the heuristics for inlining decisions.", - choices = List("at-inline-annotated", "everything"), - default = "at-inline-annotated") + choices = List("at-inline-annotated", "everything", "default"), + default = "default") object YoptWarningsChoices extends MultiChoiceEnumeration { val none = Choice("none" , "No optimizer warnings.") diff --git a/test/files/run/mapConserve.scala b/test/files/run/mapConserve.scala index c17754283a..95cad69954 100644 --- a/test/files/run/mapConserve.scala +++ b/test/files/run/mapConserve.scala @@ -1,5 +1,5 @@ /* - * filter: inliner warnings; re-run with + * filter: inliner warning */ import scala.annotation.tailrec import scala.collection.mutable.ListBuffer diff --git a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala index 622715ab3a..8429a583b5 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala @@ -1101,4 +1101,37 @@ class InlinerTest extends ClearAfterClass { assertInvoke(getSingleMethod(c, "g"), "C", "f") // g itself still has the call to f assert(r.isEmpty, r) } + + /** + * NOTE: if this test fails for you when running within the IDE, it's probably because you're + * using 2.12.0-M2 for compilining within the IDE, which doesn't add SAM information to the + * InlineInfo attribute. So the InlineInfo in the classfile for Function1 doesn't say that + * it's a SAM type. The test passes when running with ant (which does a full bootstrap). + */ + @Test + def inlineHigherOrder(): Unit = { + val code = + """class C { + | final def h(f: Int => Int): Int = f(0) + | def t1 = h(x => x + 1) + | def t2 = { + | val fun = (x: Int) => x + 1 + | h(fun) + | } + | def t3(f: Int => Int) = h(f) + | def t4(f: Int => Int) = { + | val fun = f + | h(fun) + | } + | def t5 = h(Map(0 -> 10)) // not currently inlined + |} + """.stripMargin + + val List(c) = compile(code) + assertInvoke(getSingleMethod(c, "t1"), "C", "C$$$anonfun$1") + assertInvoke(getSingleMethod(c, "t2"), "C", "C$$$anonfun$2") + assertInvoke(getSingleMethod(c, "t3"), "scala/Function1", "apply$mcII$sp") + assertInvoke(getSingleMethod(c, "t4"), "scala/Function1", "apply$mcII$sp") + assertInvoke(getSingleMethod(c, "t5"), "C", "h") + } } -- cgit v1.2.3