[vpm] common sub-expression elimination for conditions

TreeMakers (esp. CondTreeMakers) are approximated by hash-cons'ed Conds sharing is detected for prefixes of Conds, and shared conditions are only tested once their results are stored, and repeated tests branch on the last shared condition, reusing the results from the first time they were checked a Test is 1-to-1 with a TreeMaker, but may share its Cond TODO: clean separation of the two translation strategies: - naive flatMap/orElse (for virtualization) - less-naive if-then-else (with CSE etc coming) sharing trees caused wrong bytecode to be emitted (verifyerror) tentative explanation: "because lambdalift uses mutable state to track which variables have been captured if you refer to the same variable with the same tree twice it'll get confused" Sent at 8:27 PM on Thursday >> grzegorz.kossakowski: so we found a bug in jvm according to http://java.sun.com/docs/books/jvms/second_edition/html/Instructions2.doc2.html checkcast should throw a classcastexception becuase it's a shorthand for if !(x instanceof T) throw ClassCastExcpt but jvm decided to throw verifyerror and yeah, the check is wrong if jvm was not throwing verifyerror it would throw classcast exception saying that ObjectRef cannot be casted to $colon$colon ... >> me: so now where does it come from? since a ref is involved, i thought LambdaLift >> grzegorz.kossakowski: yup or now I don't think lambalift introduces that kind of low-level casts but I might be wrong btw. it's interesting that it unpacks stuff from objectref twice in your code and in one place checkcast is correct and in another is wrong Sent at 9:33 PM on Thursday >> grzegorz.kossakowski: also, since it's a verifyerror I think genjvm should have an assertion >> grzegorz.kossakowski: 193: getfield scala#54; //Field scala/runtime/ObjectRef.elem:Ljava/lang/Object; 196: checkcast #8; //class scala/runtime/ObjectRef 199: invokevirtual scala#95; //Method scala/collection/immutable/$colon$colon.tl$1:()Lscala/collection/immutable/List; it's this see you have checkcast for ObjectRef and then on that value, you try to call tl() method from List Sent at 9:56 PM on Thursday >> me: fixed sharing trees is bad very bad because lambdalift uses mutable state to track which variables have been captured if you refer to the same variable with the same tree twice it'll get confused
axel22 · Dec 24, 2011 · e0b8877 · e0b8877
1 parent 08ec6ba
commit e0b8877
Show file tree

Hide file tree

Showing 5 changed files with 456 additions and 81 deletions.
diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
@@ -290,7 +290,26 @@ abstract class UnCurry extends InfoTransform
           val idparam = m.paramss.head.head
           val substParam = new TreeSymSubstituter(List(vparam), List(idparam))
           def substTree[T <: Tree](t: T): T = substParam(resetLocalAttrs(t))
-
+          object VirtPatmatOpt {
+            object Last {
+              def unapply[T](xs: List[T]) = xs.lastOption
+            }
+            // keep this in synch by what's generated by combineCases/runOrElse
+            object MatcherBlock {
+              def unapply(matcher: Tree): Option[(ValDef, ValDef, ValDef, ValDef, List[Tree])] = matcher match { // TODO: BUG the unapplySeq version of the case below does not seem to work in virtpatmat??
+                case Block((zero: ValDef) :: (x: ValDef) :: (matchRes: ValDef) :: (keepGoing: ValDef) :: stats, _) => Some(zero, x, matchRes, keepGoing, stats)
+                case _ => None
+              }
+            }
+            // TODO: virtpatmat use case: would be nice if could abstract over the repeated pattern more easily
+            // case Block(Last(P)) =>
+            // case P =>
+            def unapply(matcher: Tree): Option[(ValDef, ValDef, ValDef, ValDef, List[Tree], Tree => Tree)] = matcher match {
+              case MatcherBlock(zero, x, matchRes, keepGoing, stats) => Some(zero, x, matchRes, keepGoing, stats, identity[Tree])
+              case Block(outerStats, MatcherBlock(zero, x, matchRes, keepGoing, stats)) => Some(zero, x, matchRes, keepGoing, stats, inner => Block(outerStats, inner))
+              case b => treeBrowser browse b; None
+            }
+          }
           DefDef(m, (fun.body: @unchecked) match {
             case Match(selector, cases) =>
               def transformCase(cdef: CaseDef): CaseDef =
@@ -301,6 +320,7 @@ abstract class UnCurry extends InfoTransform
                 if (cases exists treeInfo.isDefaultCase) Literal(Constant(true))
                 else Match(substTree(selector.duplicate), (cases map transformCase) :+ defaultCase)
               )
+            // TODO: find a better way to keep this in synch with the code generard by patmatvirtualizer
             // TODO: check tgt.tpe.typeSymbol isNonBottomSubclass MatchingStrategyClass
             case Apply(Apply(TypeApply(Select(tgt, nme.runOrElse), targs), args_scrut), args_pm) if opt.virtPatmat =>
               object noOne extends Transformer {
@@ -317,7 +337,7 @@ abstract class UnCurry extends InfoTransform
               }
               substTree(Apply(Apply(TypeApply(Select(tgt.duplicate, tgt.tpe.member("isSuccess".toTermName)), targs map (_.duplicate)), args_scrut map (_.duplicate)), args_pm map (noOne.transform)))
             // for no-option version of virtpatmat
-            case Block(List(zero: ValDef, x: ValDef, matchRes: ValDef, keepGoing: ValDef, stats@_*), _) if opt.virtPatmat => import CODE._
+            case VirtPatmatOpt(zero, x, matchRes, keepGoing, stats, addOuter) if opt.virtPatmat => import CODE._
               object dropMatchResAssign extends Transformer {
                 // override val treeCopy = newStrictTreeCopier // will duplicate below
                 override def transform(tree: Tree): Tree = tree match {
@@ -329,14 +349,14 @@ abstract class UnCurry extends InfoTransform
                 }
               }
               val statsNoMatchRes: List[Tree] = stats map (dropMatchResAssign.transform) toList
-              val idaBlock = Block(
+              val idaBlock = addOuter(Block(
                 zero ::
                 x ::
                 /* drop matchRes def */
                 keepGoing ::
                 statsNoMatchRes,
                 NOT(REF(keepGoing.symbol)) // replace `if (keepGoing) throw new MatchError(...) else matchRes` by `!keepGoing`
-              )
+              ))
               substTree(idaBlock.duplicate) // duplicate on block as a whole to ensure valdefs are properly cloned and substed
           })
         }