Merge pull request #355 from hkapp/topic/fix-global-box

Add domination checks to GlobalBoxingElimination
scala-native · Oct 26, 2016 · 40ce593 · 40ce593
2 parents fe572f1 + 6e063ef
commit 40ce593
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 10 deletions.
diff --git a/tools/src/main/scala/scala/scalanative/compiler/Compiler.scala b/tools/src/main/scala/scala/scalanative/compiler/Compiler.scala
@@ -13,7 +13,7 @@ final class Compiler(opts: Opts) {
     Global.Member(Global.Top(opts.entry), "main_class.ssnr.ObjectArray_unit")
 
   private lazy val passCompanions: Seq[PassCompanion] = Seq(
-    pass.LocalBoxingElimination,
+    pass.GlobalBoxingElimination,
     pass.DeadCodeElimination,
     pass.MainInjection,
     pass.ExternHoisting,

diff --git a/...ompiler/pass/LocalBoxingElimination.scala → ...mpiler/pass/GlobalBoxingElimination.scala b/...ompiler/pass/LocalBoxingElimination.scala → ...mpiler/pass/GlobalBoxingElimination.scala
@@ -4,28 +4,61 @@ package pass
 
 import scala.collection.mutable
 import nir._, Inst.Let
+import analysis.DominatorTree
+import analysis.ControlFlow
 
 /** Eliminates redundant box/unbox operations within
- *  a single basic block. This is quite simplistic approach
+ *  a single method definition. This is quite simplistic approach
  *  but we need this to remove boxing around pointer operations
  *  that happen to have generic signatures.
  */
-class LocalBoxingElimination extends Pass {
-  import LocalBoxingElimination._
+class GlobalBoxingElimination extends Pass {
+  import GlobalBoxingElimination._
 
   override def preDefn = {
     case defn: Defn.Define =>
       val records = mutable.UnrolledBuffer.empty[Record]
 
+      // Setup the dominator tree checks
+      val cfg             = ControlFlow.Graph(defn.insts)
+      val blockDomination = DominatorTree.build(cfg)
+
+      val localToBlock =
+        cfg.all.flatMap { block =>
+          val params = block.params.map { local =>
+            (local.name, block)
+          }
+          val insts = block.insts.collect {
+            case Let(name, _) => (name, block)
+          }
+          params ++ insts
+        }.toMap
+
+      def isDominatedBy(dominated: Local, dominating: Local): Boolean = {
+        val dominatedBlock  = localToBlock(dominated)
+        val dominatingBlock = localToBlock(dominating)
+        blockDomination(dominatedBlock).contains(dominatingBlock)
+      }
+
+      def canReuse(target: Local, reusedVal: Val): Boolean = {
+        reusedVal match {
+          case Val.Local(reusedLocal, _) => isDominatedBy(target, reusedLocal)
+          case _                         => false
+        }
+      }
+
+      // Original box elimination code
       val newinsts = defn.insts.map {
         case inst @ Let(to, op @ Op.Call(_, BoxRef(code), Seq(_, from))) =>
           records.collectFirst {
             // if a box for given value already exists, re-use the box
-            case Box(rcode, rfrom, rto) if rcode == code && from == rfrom =>
+            case Box(rcode, rfrom, rto)
+                if rcode == code && from == rfrom && canReuse(to, rto) =>
               Let(to, Op.Copy(rto))
 
             // if we re-box previously unboxed value, re-use the original box
-            case Unbox(rcode, rfrom, rto) if rcode == code && from == rto =>
+            case Unbox(rcode, rfrom, rto)
+                if rcode == code && from == rto && canReuse(to, rfrom) =>
               Let(to, Op.Copy(rfrom))
           }.getOrElse {
             // otherwise do actual boxing
@@ -36,11 +69,13 @@ class LocalBoxingElimination extends Pass {
         case inst @ Let(to, op @ Op.Call(_, UnboxRef(code), Seq(_, from))) =>
           records.collectFirst {
             // if we unbox previously boxed value, return original value
-            case Box(rcode, rfrom, rto) if rcode == code && from == rto =>
+            case Box(rcode, rfrom, rto)
+                if rcode == code && from == rto && canReuse(to, rfrom) =>
               Let(to, Op.Copy(rfrom))
 
             // if an unbox for this value already exists, re-use unbox
-            case Unbox(rcode, rfrom, rto) if rcode == code && from == rfrom =>
+            case Unbox(rcode, rfrom, rto)
+                if rcode == code && from == rfrom && canReuse(to, rto) =>
               Let(to, Op.Copy(rto))
           }.getOrElse {
             // otherwise do actual unboxing
@@ -56,7 +91,7 @@ class LocalBoxingElimination extends Pass {
   }
 }
 
-object LocalBoxingElimination extends PassCompanion {
+object GlobalBoxingElimination extends PassCompanion {
   private sealed abstract class Record
   private final case class Box(code: Char, from: nir.Val, to: nir.Val)
       extends Record
@@ -115,5 +150,5 @@ object LocalBoxingElimination extends PassCompanion {
     }
   }
 
-  def apply(ctx: Ctx) = new LocalBoxingElimination
+  def apply(ctx: Ctx) = new GlobalBoxingElimination
 }