Fixed various issues with Icode, mainly fixing ...

Fixed various issues with Icode, mainly fixing empty/open blocks.

7 files changed, 66 insertions, 41 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala b/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala
index bab3652f69..a5e2e978a3 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala
@@ -93,7 +93,7 @@ trait BasicBlocks {
     override def toList: List[Instruction] = {
       if (closed)
         instrs.toList
-      else instructionList
+      else instructionList.reverse
     }
 
     /** Return an iterator over the instructions in this basic block. */
@@ -131,8 +131,8 @@ trait BasicBlocks {
     /** Apply a function to all the instructions of the block. */
     override def foreach[U](f: Instruction => U) = {
       if (!closed) {
-        dump
-        global.abort("Traversing an open block!: " + label)
+        method.dump
+        global.abort("Traversing an open block!: " + label + " in " + method)
       }
       instrs foreach f
     }
@@ -368,8 +368,24 @@ trait BasicBlocks {
       this.close
     }
 
+    /** do nothing if block is already closed */
+    def closeWith(instr: Instruction) {
+      if (closed) () else {
+        emit(instr)
+        close
+      }
+    }
+
+    def closeWith(instr: Instruction, pos: Position) {
+      if (closed) () else {
+        emit(instr, pos)
+        close
+      }
+    }
+
     /** Close the block */
     def close {
+      assert(!closed)
       assert(instructionList.length > 0, "Empty block.")
       closed = true
       setFlag(DIRTYSUCCS)
diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
index 9136bffcda..27bf9ac29b 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -134,11 +134,11 @@ abstract class GenICode extends SubComponent  {
             case EmptyTree =>
               error("Concrete method has no definition: " + tree)
             case _ => if (ctx1.bb.isEmpty)
-              ctx1.bb.emit(RETURN(m.returnType), rhs.pos)
+              ctx1.bb.closeWith(RETURN(m.returnType), rhs.pos)
             else
-              ctx1.bb.emit(RETURN(m.returnType))
+              ctx1.bb.closeWith(RETURN(m.returnType))
           }
-          ctx1.bb.close
+          if (!ctx1.bb.closed) ctx1.bb.close
           prune(ctx1.method)
         } else
           ctx1.method.setCode(null)
@@ -373,11 +373,12 @@ abstract class GenICode extends SubComponent  {
       assert(!settings.debug.value || !(hasUnitBranch && expectedType != UNIT),
         "I produce UNIT in a context where " + expectedType + " is expected!")
 
-      thenCtx.bb.emitOnly(JUMP(contCtx.bb))
-      elseCtx.bb.emitOnly(
-        if (elsep == EmptyTree) JUMP(contCtx.bb)
-        else JUMP(contCtx.bb) setPos tree.pos
-      )
+      // alternatives may be already closed by a tail-recursive jump
+      thenCtx.bb.closeWith(JUMP(contCtx.bb))
+      elseCtx.bb.closeWith(
+          if (elsep == EmptyTree) JUMP(contCtx.bb)
+          else JUMP(contCtx.bb) setPos tree.pos
+        )
 
       (contCtx, resKind)
     }
@@ -489,8 +490,7 @@ abstract class GenICode extends SubComponent  {
                 log("Adding label " + tree.symbol);
           }
 
-          ctx.bb.emit(JUMP(ctx1.bb), tree.pos)
-          ctx.bb.close
+          ctx.bb.closeWith(JUMP(ctx1.bb), tree.pos)
           genLoad(rhs, ctx1, expectedType /*toTypeKind(tree.symbol.info.resultType)*/)
 
         case ValDef(_, nme.THIS, _, _) =>
@@ -719,7 +719,8 @@ abstract class GenICode extends SubComponent  {
             }
             val ctx1 = genLoadLabelArguments(args, label, ctx)
             ctx1.bb.emitOnly(if (label.anchored) JUMP(label.block) else PJUMP(label))
-            ctx1.newBlock
+            ctx1.bb.enterIgnoreMode
+            ctx1
           } else if (isPrimitive(sym)) { // primitive method call
             val (newCtx, resKind) = genPrimitiveOp(app, ctx, expectedType)
             generatedType = resKind
@@ -925,7 +926,7 @@ abstract class GenICode extends SubComponent  {
             }
 
             caseCtx = genLoad(body, tmpCtx, generatedType)
-            caseCtx.bb.emitOnly(JUMP(afterCtx.bb) setPos caze.pos)
+            caseCtx.bb.closeWith(JUMP(afterCtx.bb) setPos caze.pos)
           }
           ctx1.bb.emitOnly(
             SWITCH(tags.reverse map (x => List(x)), (default :: targets).reverse) setPos tree.pos
@@ -1291,7 +1292,7 @@ abstract class GenICode extends SubComponent  {
       // the default emission
       def default = {
         val ctx1 = genLoad(tree, ctx, BOOL)
-        ctx1.bb.emitOnly(CZJUMP(thenCtx.bb, elseCtx.bb, NE, BOOL) setPos tree.pos)
+        ctx1.bb.closeWith(CZJUMP(thenCtx.bb, elseCtx.bb, NE, BOOL) setPos tree.pos)
       }
 
       tree match {
@@ -1926,8 +1927,7 @@ abstract class GenICode extends SubComponent  {
 
         def emitFinalizer(ctx: Context): Context = if (!finalizer.isEmpty) {
           val ctx1 = finalizerCtx.dup.newBlock
-          ctx.bb.emit(JUMP(ctx1.bb))
-          ctx.bb.close
+          ctx.bb.closeWith(JUMP(ctx1.bb))
 
           if (guardResult) {
             ctx1.bb.emit(STORE_LOCAL(tmp))
@@ -1962,8 +1962,7 @@ abstract class GenICode extends SubComponent  {
             ctx1 = handler._3(ctx1)
             // emit finalizer
             val ctx2 = emitFinalizer(ctx1)
-            ctx2.bb.emit(JUMP(afterCtx.bb))
-            ctx2.bb.close
+            ctx2.bb.closeWith(JUMP(afterCtx.bb))
             outerCtx.endHandler()
             exh
           }
@@ -1974,11 +1973,9 @@ abstract class GenICode extends SubComponent  {
         var finalCtx = body(bodyCtx)
         finalCtx = emitFinalizer(finalCtx)
 
-        outerCtx.bb.emit(JUMP(bodyCtx.bb))
-        outerCtx.bb.close
+        outerCtx.bb.closeWith(JUMP(bodyCtx.bb))
 
-        finalCtx.bb.emit(JUMP(afterCtx.bb))
-        finalCtx.bb.close
+        finalCtx.bb.closeWith(JUMP(afterCtx.bb))
 
         afterCtx
       }
@@ -2018,8 +2015,7 @@ abstract class GenICode extends SubComponent  {
           if (settings.Xdce.value) ctx.bb.emit(LOAD_EXCEPTION())
           val ctx1 = genLoad(finalizer, ctx, UNIT)
           // need jump for the ICode to be valid. MSIL backend will emit `Endfinally` instead.
-          ctx1.bb.emit(JUMP(afterCtx.bb))
-          ctx1.bb.close
+          ctx1.bb.closeWith(JUMP(afterCtx.bb))
           finalizerCtx.endHandler()
         }
 
@@ -2029,8 +2025,7 @@ abstract class GenICode extends SubComponent  {
           if (settings.Xdce.value) ctx1.bb.emit(LOAD_EXCEPTION())
           ctx1 = handler._3(ctx1)
           // msil backend will emit `Leave` to jump out of a handler
-          ctx1.bb.emit(JUMP(afterCtx.bb))
-          ctx1.bb.close
+          ctx1.bb.closeWith(JUMP(afterCtx.bb))
           outerCtx.endHandler()
         }
 
@@ -2038,12 +2033,10 @@ abstract class GenICode extends SubComponent  {
 
         val finalCtx = body(bodyCtx)
 
-        outerCtx.bb.emit(JUMP(bodyCtx.bb))
-        outerCtx.bb.close
+        outerCtx.bb.closeWith(JUMP(bodyCtx.bb))
 
         // msil backend will emit `Leave` to jump out of a try-block
-        finalCtx.bb.emit(JUMP(afterCtx.bb))
-        finalCtx.bb.close
+        finalCtx.bb.closeWith(JUMP(afterCtx.bb))
 
         afterCtx
       }
diff --git a/src/compiler/scala/tools/nsc/backend/icode/ICodes.scala b/src/compiler/scala/tools/nsc/backend/icode/ICodes.scala
index dc90a438b9..30aaebc3c2 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/ICodes.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/ICodes.scala
@@ -71,6 +71,14 @@ abstract class ICodes extends AnyRef
     classes.values foreach printer.printClass
   }
 
+  def checkValid(m: IMethod) {
+    for (b <- m.code.blocks)
+      if (!b.closed) {
+        m.dump
+        global.abort("Open block: " + b.flagsString)
+      }
+  }
+
   object liveness extends Liveness {
     val global: ICodes.this.global.type = ICodes.this.global
   }
diff --git a/src/compiler/scala/tools/nsc/backend/icode/Members.scala b/src/compiler/scala/tools/nsc/backend/icode/Members.scala
index 95cd15711b..32a3c550ed 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/Members.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/Members.scala
@@ -253,6 +253,7 @@ trait Members { self: ICodes =>
         } else
           bb = nextBlock.keysIterator.next
       }
+      checkValid(this)
     }
 
     def dump {
diff --git a/src/compiler/scala/tools/nsc/backend/icode/Printers.scala b/src/compiler/scala/tools/nsc/backend/icode/Printers.scala
index 9aaeeb2e8b..a9ec70d390 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/Printers.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/Printers.scala
@@ -120,7 +120,7 @@ trait Printers { self: ICodes =>
       print(": ");
       if (settings.debug.value) print("pred: " + bb.predecessors + " succs: " + bb.successors + " flags: " + bb.flagsString)
       indent; println
-      bb foreach printInstruction
+      bb.toList foreach printInstruction
       undent; println
     }
 
diff --git a/src/compiler/scala/tools/nsc/backend/icode/analysis/Liveness.scala b/src/compiler/scala/tools/nsc/backend/icode/analysis/Liveness.scala
index 101bb81503..b1d22a12f0 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/analysis/Liveness.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/analysis/Liveness.scala
@@ -49,7 +49,7 @@ abstract class Liveness {
       gen.clear
       kill.clear
 
-      for (b <- m.code.blocks.toList;
+      for (b <- m.code.blocks;
            (g, k) = genAndKill(b)) {
         gen  += (b -> g)
         kill += (b -> k)
diff --git a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala
index 9650dff7c3..155bc17491 100644
--- a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala
+++ b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala
@@ -125,7 +125,8 @@ abstract class Inliners extends SubComponent {
       val caller = new IMethodInfo(m)
       var info: tfa.lattice.Elem = null
 
-      def analyzeInc(msym: Symbol, i: Instruction, bb: BasicBlock) = {
+      def analyzeInc(msym: Symbol, i: Instruction, bb: BasicBlock): Boolean = {
+        var inlined = false
         def paramTypes  = msym.info.paramTypes
         val receiver    = (info.stack.types drop paramTypes.length).head match {
           case REFERENCE(s) => s
@@ -164,6 +165,7 @@ abstract class Inliners extends SubComponent {
 
               if (pair isStampedForInlining info.stack) {
                 retry = true
+                inlined = true
                 if (isCountable)
                   count += 1
 
@@ -191,8 +193,10 @@ abstract class Inliners extends SubComponent {
           if (!isAvailable) "bytecode was not available"
           else "it is not final"
         )
+        inlined
       }
 
+      import scala.util.control.Breaks._
       do {
         retry = false
         if (caller.inline) {
@@ -205,11 +209,13 @@ abstract class Inliners extends SubComponent {
           caller.linearized foreach { bb =>
             info = tfa in bb
 
-            for (i <- bb) {
-              if (!retry) {
+            breakable {
+              for (i <- bb) {
                 i match {
-                  case CALL_METHOD(msym, Dynamic) => analyzeInc(msym, i, bb)
-                  case _                          => ()
+                  case CALL_METHOD(msym, Dynamic) =>
+                    if (analyzeInc(msym, i, bb)) break
+
+                  case _ => ()
                 }
                 info = tfa.interpret(info, i)
               }
@@ -497,6 +503,7 @@ abstract class Inliners extends SubComponent {
         // add exception handlers of the callee
         caller addHandlers (inc.handlers map translateExh)
         assert(pending.isEmpty, "Pending NEW elements: " + pending)
+        if (settings.debug.value) icodes.checkValid(caller.m)
       }
 
       def isStampedForInlining(stack: TypeStack) =
@@ -595,8 +602,8 @@ abstract class Inliners extends SubComponent {
 
           false
         }
-        if (!canAccess(accessNeeded))
-          println("access needed and failed: " + accessNeeded)
+//        if (!canAccess(accessNeeded))
+//          println("access needed and failed: " + accessNeeded)
         canAccess(accessNeeded) && !isIllegalStack
       }