Skip to content

Commit

Permalink
SI-8062 Fix inliner cycle with recursion, separate compilation
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
retronym committed Dec 10, 2013
1 parent 7e996c1 commit f0d913b
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 5 deletions.
5 changes: 1 addition & 4 deletions src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/scala/tools/nsc/backend/icode/Members.scala
Expand Up @@ -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 =
Expand Down
Expand Up @@ -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.")
Expand Down
1 change: 1 addition & 0 deletions test/files/pos/t8062.flags
@@ -0,0 +1 @@
-optimize
5 changes: 5 additions & 0 deletions 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)
}
3 changes: 3 additions & 0 deletions test/files/pos/t8062/B_2.scala
@@ -0,0 +1,3 @@
object Test {
warmup.Warmup.filter[Any](x => false)
}

0 comments on commit f0d913b

Please sign in to comment.