diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2013-12-10 11:04:26 +0100 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2013-12-10 11:11:57 +0100 |
commit | f0d913b51dbe1a098e813aa08197eef9c6cbf737 (patch) | |
tree | 59e8343f0411b9c676f0a94d1174f5132e71f092 | |
parent | 7e996c1b05902df0144709a37d9149252885495c (diff) | |
download | scala-f0d913b51dbe1a098e813aa08197eef9c6cbf737.tar.gz scala-f0d913b51dbe1a098e813aa08197eef9c6cbf737.tar.bz2 scala-f0d913b51dbe1a098e813aa08197eef9c6cbf737.zip |
SI-8062 Fix inliner cycle with recursion, separate compilation
ICodeReaders, which decompiles JVM bytecode to ICode, was not
setting the `recursive` attribute of `IMethod`. This meant that
the inliner got into a cycle, repeatedly inlining the recursive
call.
The method name `filter` was needed to trigger this as the inliner
heuristically treats that as a more attractive inlining candidate,
based on `isMonadicMethod`.
This commit:
- refactors the checking / setting of `virtual`
- adds this to ICodeReaders
- tests the case involving `invokevirtual`
I'm not sure how to setup a test that fails without the other changes
to `ICodeReader` (for invokestatic and invokespecial).
-rw-r--r-- | src/compiler/scala/tools/nsc/backend/icode/GenICode.scala | 5 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/backend/icode/Members.scala | 4 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala | 7 | ||||
-rw-r--r-- | test/files/pos/t8062.flags | 1 | ||||
-rw-r--r-- | test/files/pos/t8062/A_1.scala | 5 | ||||
-rw-r--r-- | test/files/pos/t8062/B_2.scala | 3 |
6 files changed, 20 insertions, 5 deletions
diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala index 44d7a1929b..71a5b85271 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala @@ -954,10 +954,7 @@ abstract class GenICode extends SubComponent { case _ => } ctx1.bb.emit(cm, tree.pos) - - if (sym == ctx1.method.symbol) { - ctx1.method.recursive = true - } + ctx1.method.updateRecursive(sym) generatedType = if (sym.isClassConstructor) UNIT else toTypeKind(sym.info.resultType); diff --git a/src/compiler/scala/tools/nsc/backend/icode/Members.scala b/src/compiler/scala/tools/nsc/backend/icode/Members.scala index b74770f051..00bcf603cf 100644 --- a/src/compiler/scala/tools/nsc/backend/icode/Members.scala +++ b/src/compiler/scala/tools/nsc/backend/icode/Members.scala @@ -185,6 +185,10 @@ trait Members { this } + final def updateRecursive(called: Symbol): Unit = { + recursive ||= (called == symbol) + } + def addLocal(l: Local): Local = findOrElse(locals)(_ == l) { locals ::= l ; l } def addParam(p: Local): Unit = diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala index c304c18c4f..d0c540a2c6 100644 --- a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala +++ b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala @@ -489,23 +489,28 @@ abstract class ICodeReader extends ClassfileParser { case JVM.invokevirtual => val m = pool.getMemberSymbol(in.nextChar, false); size += 2 code.emit(CALL_METHOD(m, Dynamic)) + method.updateRecursive(m) case JVM.invokeinterface => val m = pool.getMemberSymbol(in.nextChar, false); size += 4 in.skip(2) code.emit(CALL_METHOD(m, Dynamic)) + // invokeinterface can't be recursive case JVM.invokespecial => val m = pool.getMemberSymbol(in.nextChar, false); size += 2 val style = if (m.name == nme.CONSTRUCTOR || m.isPrivate) Static(true) else SuperCall(m.owner.name); code.emit(CALL_METHOD(m, style)) + method.updateRecursive(m) case JVM.invokestatic => val m = pool.getMemberSymbol(in.nextChar, true); size += 2 if (isBox(m)) code.emit(BOX(toTypeKind(m.info.paramTypes.head))) else if (isUnbox(m)) code.emit(UNBOX(toTypeKind(m.info.resultType))) - else + else { code.emit(CALL_METHOD(m, Static(false))) + method.updateRecursive(m) + } case JVM.invokedynamic => // TODO, this is just a place holder. A real implementation must parse the class constant entry debuglog("Found JVM invokedynamic instructionm, inserting place holder ICode INVOKE_DYNAMIC.") diff --git a/test/files/pos/t8062.flags b/test/files/pos/t8062.flags new file mode 100644 index 0000000000..49d036a887 --- /dev/null +++ b/test/files/pos/t8062.flags @@ -0,0 +1 @@ +-optimize diff --git a/test/files/pos/t8062/A_1.scala b/test/files/pos/t8062/A_1.scala new file mode 100644 index 0000000000..ca0411dae8 --- /dev/null +++ b/test/files/pos/t8062/A_1.scala @@ -0,0 +1,5 @@ +package warmup + +object Warmup { + def filter[A](p: Any => Boolean): Any = filter[Any](p) +} diff --git a/test/files/pos/t8062/B_2.scala b/test/files/pos/t8062/B_2.scala new file mode 100644 index 0000000000..f0a6761488 --- /dev/null +++ b/test/files/pos/t8062/B_2.scala @@ -0,0 +1,3 @@ +object Test { + warmup.Warmup.filter[Any](x => false) +} |