Optimize unnecessary boxing/unboxing.

5 files changed, 62 insertions, 21 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/icode/analysis/CopyPropagation.scala b/src/compiler/scala/tools/nsc/backend/icode/analysis/CopyPropagation.scala
index eda889ae76..6569ec29af 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/analysis/CopyPropagation.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/analysis/CopyPropagation.scala
@@ -20,7 +20,7 @@ abstract class CopyPropagation {
   import global._
   import icodes._
 
-  /** Locations can be local variables. */
+  /** Locations can be local variables and fields. */
   abstract sealed class Location;
   case class LocalVar(l: Local) extends Location
   case class Field(r: Record, sym: Symbol) extends Location
@@ -34,6 +34,7 @@ abstract class CopyPropagation {
     override def isRecord = true
   }
   case class Deref(l: Location) extends Value;
+  case class Boxed(l: Location) extends Value;
   case object Unknown extends Value
   object AllRecords extends Record(NoSymbol, new HashMap[Symbol, Value])
 
@@ -58,7 +59,10 @@ abstract class CopyPropagation {
           }
         }
 
-      /* Return the binding for the given local (another local) */
+      /* Return an alias for the given local. It returns the last
+       * local in the chain of aliased locals. Cycles are not allowed
+       * to exist (by construction).
+       */
       def getAlias(l: Local): Local = {
         var target = l
         var stop = false
@@ -72,7 +76,7 @@ abstract class CopyPropagation {
         target
       }
 
-      /* Return the binding for the given local. */
+      /* Return the value bound to the given local. */
       def getBinding(l: Local): Value = {
         var target = l
         var stop = false
@@ -89,8 +93,6 @@ abstract class CopyPropagation {
         value
       }
 
-
-
       /* Return the binding for the given field of the given record */
       def getBinding(r: Record, f: Symbol): Value = {
         assert(r.bindings.isDefinedAt(f),
@@ -309,10 +311,20 @@ abstract class CopyPropagation {
         }
 
         case BOX(tpe) =>
-          out.stack = Unknown :: out.stack.drop(1)
+          val top = out.stack.head
+          top match {
+            case Deref(loc) =>
+              out.stack = Boxed(loc) :: out.stack.tail
+            case _ =>
+              out.stack = Unknown :: out.stack.drop(1)
+          }
 
         case UNBOX(tpe) =>
-          out.stack = Unknown :: out.stack.drop(1)
+          val top = out.stack.head
+          top match {
+            case Boxed(loc) => Deref(loc) :: out.stack.tail
+            case _          => out.stack = Unknown :: out.stack.drop(1)
+          }
 
         case NEW(kind) =>
           val v1 =
@@ -390,6 +402,7 @@ abstract class CopyPropagation {
         r.bindings retain { (loc, value) =>
           value match {
             case Deref(loc1) if (loc1 == target) => false
+            case Boxed(loc1) if (loc1 == target)  => false
             case _ => true
           }
         }
@@ -399,12 +412,14 @@ abstract class CopyPropagation {
       s.stack = s.stack map { v => v match {
         case Record(_, bindings) =>
           cleanRecord(v.asInstanceOf[Record])
+        case Boxed(loc1) if (loc1 == target) => Unknown
         case _ => v
       }}
 
       s.bindings retain { (loc, value) =>
         (value match {
           case Deref(loc1) if (loc1 == target) => false
+          case Boxed(loc1) if (loc1 == target) => false
           case Record(_, _) =>
             cleanRecord(value.asInstanceOf[Record]);
             true
@@ -450,6 +465,7 @@ abstract class CopyPropagation {
       state.bindings retain {(loc, value) =>
         value match {
           case Deref(Field(_, _)) => false
+          case Boxed(Field(_, _)) => false
           case _ => true
         }
       }
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
index 0203d73f39..621db1a067 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
@@ -41,6 +41,9 @@ abstract class GenJVM extends SubComponent {
 
     override def run: Unit = {
       if (settings.debug.value) inform("[running phase " + name + " on icode]")
+      if (settings.Xdce.value)
+        icodes.classes.retain { (sym: Symbol, cls: IClass) => !inliner.isClosureClass(sym) || deadCode.liveClosures(sym) }
+
       classes.values foreach codeGenerator.genClass
     }
 
diff --git a/src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala b/src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala
index 63c865b79e..7defa675b4 100644
--- a/src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala
+++ b/src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala
@@ -67,6 +67,9 @@ abstract class ClosureElimination extends SubComponent {
         case (LOAD_LOCAL(_), DROP(_)) =>
           Some(Nil)
 
+        case (BOX(t1), UNBOX(t2)) if (t1 == t2) =>
+          Some(Nil)
+
         case (LOAD_FIELD(sym, isStatic), DROP(_)) =>
         	if (isStatic)
             Some(Nil)
@@ -143,6 +146,21 @@ abstract class ClosureElimination extends SubComponent {
                 case _ => ();
               }
 
+            case UNBOX(_) =>
+              info.stack match {
+                case Deref(LocalVar(loc1)) :: _ if (info.bindings.isDefinedAt(LocalVar(loc1))) =>
+                  val value = info.getBinding(loc1)
+                  value match {
+                    case Boxed(LocalVar(loc2)) =>
+                      bb.replaceInstruction(i, DROP(icodes.AnyRefReference) :: valueToInstruction(info.getBinding(loc2)) :: Nil)
+                      log("replaced " + i + " with " + info.getBinding(loc2))
+                    case _ =>
+                      ()
+                  }
+                case _ =>
+                  ()
+              }
+
             case _ => ();
           }
           info = cpp.interpret(info, i)
diff --git a/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala b/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala
index c36b8b4d42..496051ea99 100644
--- a/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala
+++ b/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala
@@ -36,12 +36,14 @@ abstract class DeadCodeElimination extends SubComponent {
         dce.analyzeClass(c)
   }
 
+  /** closures that are instantiated at least once, after dead code elimination */
+  val liveClosures: mutable.Set[Symbol] = new mutable.HashSet()
+
   /** Remove dead code.
    */
   class DeadCode {
 
     def analyzeClass(cls: IClass): Unit = {
-      liveClosures.clear
       cls.methods.foreach { m =>
         this.method = m
 //        analyzeMethod(m);
@@ -63,9 +65,6 @@ abstract class DeadCodeElimination extends SubComponent {
     /** what local variables have been accessed at least once? */
     var accessedLocals: List[Local] = Nil
 
-    /** closures that are instantiated at least once, after dead code elimination */
-    val liveClosures: mutable.Set[Symbol] = new mutable.HashSet()
-
     /** the current method. */
     var method: IMethod = _
 
@@ -134,9 +133,9 @@ abstract class DeadCodeElimination extends SubComponent {
             case nw @ NEW(REFERENCE(sym)) =>
               assert(nw.init ne null, "null new.init at: " + bb + ": " + idx + "(" + instr + ")")
               worklist += findInstruction(bb, nw.init)
-              if (inliner.isClosureClass(sym))
+              if (inliner.isClosureClass(sym)) {
                 liveClosures += sym
-
+              }
             case LOAD_EXCEPTION() =>
               ()
 
@@ -176,9 +175,6 @@ abstract class DeadCodeElimination extends SubComponent {
             i match {
               case NEW(REFERENCE(sym)) =>
                 log("skipped object creation: " + sym + "inside " + m)
-                if (inliner.isClosureClass(sym) && !liveClosures(sym)) {
-                  icodes.classes -= sym
-                }
               case _ => ()
             }
             if (settings.debug.value) log("Skipped: bb_" + bb + ": " + idx + "( " + i + ")")
diff --git a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala
index 4f5fce2258..135aa74a7f 100644
--- a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala
+++ b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala
@@ -321,7 +321,7 @@ abstract class Inliners extends SubComponent {
                       classes(receiver).lookupMethod(concreteMethod) match {
                         case Some(inc) =>
                           if (inc.symbol != m.symbol
-                              && (inlinedMethods(inc.symbol) < 2)
+                              //&& (inlinedMethods(inc.symbol) < 2)
                               && (inc.code ne null)
                               && shouldInline(m, inc)
                               && (inc.code.blocks.length <= MAX_INLINE_SIZE)
@@ -341,7 +341,8 @@ abstract class Inliners extends SubComponent {
                                   + "\n\t(inlinedMethods(inc.symbol) < 2): " + (inlinedMethods(inc.symbol) < 2)
                                   + "\n\tinc.code ne null: " + (inc.code ne null)
                                   + "\n\tinc.code.blocks.length < MAX_INLINE_SIZE: " + (inc.code.blocks.length < MAX_INLINE_SIZE) + "(" + inc.code.blocks.length + ")"
-                                  + "\n\tisSafeToInline(m, inc, info._2): " + isSafeToInline(m, inc, info._2));
+                                  + "\n\tisSafeToInline(m, inc, info._2): " + isSafeToInline(m, inc, info._2)
+                                  + "\n\tshouldInline heuristics: " + shouldInline(m, inc));
                           }
                         case None =>
                           ();
@@ -453,15 +454,22 @@ abstract class Inliners extends SubComponent {
    */
   def shouldInline(caller: IMethod, callee: IMethod): Boolean = {
      if (caller.symbol.hasFlag(Flags.BRIDGE)) return false;
-
+     if (settings.debug.value)
+       log("shouldInline: " + caller + " with " + callee)
      var score = 0
      if (callee.code.blocks.length <= SMALL_METHOD_SIZE) score = score + 1
      if (caller.code.blocks.length <= SMALL_METHOD_SIZE
-         && (caller.code.blocks.length + callee.code.blocks.length) < SMALL_METHOD_SIZE)
+         && ((caller.code.blocks.length + callee.code.blocks.length) > SMALL_METHOD_SIZE)) {
        score = score - 1
+       if (settings.debug.value)
+         log("shouldInline: score decreased to " + score + " because small " + caller + " would become large")
+     }
 
-     if (callee.symbol.tpe.paramTypes.exists(definitions.isFunctionType))
+     if (callee.symbol.tpe.paramTypes.exists(t => definitions.FunctionClass.contains(t.symbol))) {
+       if (settings.debug.value)
+         log("increased score to: " + score)
        score = score + 2
+     }
      if (isClosureClass(callee.symbol.owner))
        score = score + 2