diff options
| -rw-r--r-- | src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala | 2 | ||||
| -rw-r--r-- | src/compiler/scala/tools/nsc/backend/opt/Inliners.scala | 7 | ||||
| -rw-r--r-- | src/continuations/plugin/scala/tools/selectivecps/CPSAnnotationChecker.scala | 22 | ||||
| -rw-r--r-- | src/continuations/plugin/scala/tools/selectivecps/SelectiveANFTransform.scala | 14 | ||||
| -rw-r--r-- | test/files/continuations-neg/t5445.check | 4 | ||||
| -rw-r--r-- | test/files/continuations-neg/t5445.scala | 5 | ||||
| -rw-r--r-- | test/files/continuations-run/t5538.check | 1 | ||||
| -rw-r--r-- | test/files/continuations-run/t5538.scala | 50 | ||||
| -rw-r--r-- | test/files/run/test-cpp.check | 73 | ||||
| -rw-r--r-- | test/files/run/test-cpp.scala | 104 |
10 files changed, 263 insertions, 19 deletions
diff --git a/src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala b/src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala index e8abee7d06..ff45bb8fd1 100644 --- a/src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala +++ b/src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala @@ -108,7 +108,7 @@ abstract class ClosureElimination extends SubComponent { case LOAD_LOCAL(l) if info.bindings isDefinedAt LocalVar(l) => val t = info.getBinding(l) t match { - case Deref(LocalVar(_)) | Deref(This) | Const(_) => + case Deref(This) | Const(_) => bb.replaceInstruction(i, valueToInstruction(t)); log("replaced " + i + " with " + t) diff --git a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala index e91bab8367..a734b2b92b 100644 --- a/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala +++ b/src/compiler/scala/tools/nsc/backend/opt/Inliners.scala @@ -707,7 +707,8 @@ abstract class Inliners extends SubComponent { } def isStampedForInlining(stackLength: Int) = - !sameSymbols && inc.m.hasCode && shouldInline && isSafeToInline(stackLength) && !inc.m.symbol.hasFlag(Flags.SYNCHRONIZED) + !sameSymbols && inc.m.hasCode && shouldInline && + isSafeToInline(stackLength) // `isSafeToInline()` must be invoked last in this AND expr bc it mutates the `knownSafe` and `knownUnsafe` maps for good. def logFailure(stackLength: Int) = log( """|inline failed for %s: @@ -765,8 +766,8 @@ abstract class Inliners extends SubComponent { true } - if (!inc.m.hasCode || inc.isRecursive) - return false + if (!inc.m.hasCode || inc.isRecursive) { return false } + if (inc.m.symbol.hasFlag(Flags.SYNCHRONIZED)) { return false } val accessNeeded = usesNonPublics.getOrElseUpdate(inc.m, { // Avoiding crashing the compiler if there are open blocks. diff --git a/src/continuations/plugin/scala/tools/selectivecps/CPSAnnotationChecker.scala b/src/continuations/plugin/scala/tools/selectivecps/CPSAnnotationChecker.scala index 0382304bad..af0d768607 100644 --- a/src/continuations/plugin/scala/tools/selectivecps/CPSAnnotationChecker.scala +++ b/src/continuations/plugin/scala/tools/selectivecps/CPSAnnotationChecker.scala @@ -336,29 +336,31 @@ abstract class CPSAnnotationChecker extends CPSUtils { def single(xs: List[AnnotationInfo]) = xs match { case List(x) => x case _ => - global.globalError("not a single cps annotation: " + xs)// FIXME: error message + global.globalError("not a single cps annotation: " + xs) xs(0) } + + def emptyOrSingleList(xs: List[AnnotationInfo]) = if (xs.isEmpty) Nil else List(single(xs)) def transChildrenInOrder(tree: Tree, tpe: Type, childTrees: List[Tree], byName: List[Tree]) = { def inspect(t: Tree): List[AnnotationInfo] = { if (t.tpe eq null) Nil else { val extra: List[AnnotationInfo] = t.tpe match { case _: MethodType | _: PolyType | _: OverloadedType => - // method types, poly types and overloaded types do not obtain cps annotions by propagat - // need to reconstruct transitively from their children. - t match { - case Select(qual, name) => inspect(qual) - case Apply(fun, args) => (fun::args) flatMap inspect - case TypeApply(fun, args) => (fun::args) flatMap inspect - case _ => Nil - } + // method types, poly types and overloaded types do not obtain cps annotions by propagation + // need to reconstruct transitively from their children. + t match { + case Select(qual, name) => inspect(qual) + case Apply(fun, args) => (fun::(transArgList(fun,args).flatten)) flatMap inspect + case TypeApply(fun, args) => (fun::(transArgList(fun,args).flatten)) flatMap inspect + case _ => Nil + } case _ => Nil } val types = cpsParamAnnotation(t.tpe) // TODO: check that it has been adapted and if so correctly - extra ++ (if (types.isEmpty) Nil else List(single(types))) + extra ++ emptyOrSingleList(types) } } val children = childTrees flatMap inspect diff --git a/src/continuations/plugin/scala/tools/selectivecps/SelectiveANFTransform.scala b/src/continuations/plugin/scala/tools/selectivecps/SelectiveANFTransform.scala index d98169f21a..1189cc2e38 100644 --- a/src/continuations/plugin/scala/tools/selectivecps/SelectiveANFTransform.scala +++ b/src/continuations/plugin/scala/tools/selectivecps/SelectiveANFTransform.scala @@ -97,13 +97,17 @@ abstract class SelectiveANFTransform extends PluginComponent with Transform with case vd @ ValDef(mods, name, tpt, rhs) => // object-level valdefs debuglog("transforming valdef " + vd.symbol) - atOwner(vd.symbol) { + if (getExternalAnswerTypeAnn(tpt.tpe).isEmpty) { + + atOwner(vd.symbol) { - assert(getExternalAnswerTypeAnn(tpt.tpe) == None) + val rhs1 = transExpr(rhs, None, None) - val rhs1 = transExpr(rhs, None, None) - - treeCopy.ValDef(vd, mods, name, transform(tpt), rhs1) + treeCopy.ValDef(vd, mods, name, transform(tpt), rhs1) + } + } else { + unit.error(tree.pos, "cps annotations not allowed on by-value parameters or value definitions") + super.transform(tree) } case TypeTree() => diff --git a/test/files/continuations-neg/t5445.check b/test/files/continuations-neg/t5445.check new file mode 100644 index 0000000000..eb2943b6a6 --- /dev/null +++ b/test/files/continuations-neg/t5445.check @@ -0,0 +1,4 @@ +t5445.scala:4: error: cps annotations not allowed on by-value parameters or value definitions + def foo(block: Unit @suspendable ): Unit @suspendable = {} + ^ +one error found diff --git a/test/files/continuations-neg/t5445.scala b/test/files/continuations-neg/t5445.scala new file mode 100644 index 0000000000..cb6f8f686d --- /dev/null +++ b/test/files/continuations-neg/t5445.scala @@ -0,0 +1,5 @@ +import scala.util.continuations._ + +object Test { + def foo(block: Unit @suspendable ): Unit @suspendable = {} +} diff --git a/test/files/continuations-run/t5538.check b/test/files/continuations-run/t5538.check new file mode 100644 index 0000000000..457721d5e0 --- /dev/null +++ b/test/files/continuations-run/t5538.check @@ -0,0 +1 @@ +Future(Future(Future(Future(Future(List(1, 2, 3, 4, 5)))))) diff --git a/test/files/continuations-run/t5538.scala b/test/files/continuations-run/t5538.scala new file mode 100644 index 0000000000..42f8163caf --- /dev/null +++ b/test/files/continuations-run/t5538.scala @@ -0,0 +1,50 @@ +import scala.util.continuations._ +import scala.collection.generic.CanBuildFrom + +object Test { + + class ExecutionContext + + implicit def defaultExecutionContext = new ExecutionContext + + case class Future[+T](x:T) { + final def map[A](f: T => A): Future[A] = new Future[A](f(x)) + final def flatMap[A](f: T => Future[A]): Future[A] = f(x) + } + + class PromiseStream[A] { + override def toString = xs.toString + + var xs: List[A] = Nil + + final def +=(elem: A): this.type = { xs :+= elem; this } + + final def ++=(elem: Traversable[A]): this.type = { xs ++= elem; this } + + final def <<(elem: Future[A]): PromiseStream[A] @cps[Future[Any]] = + shift { cont: (PromiseStream[A] => Future[Any]) => elem map (a => cont(this += a)) } + + final def <<(elem1: Future[A], elem2: Future[A], elems: Future[A]*): PromiseStream[A] @cps[Future[Any]] = + shift { cont: (PromiseStream[A] => Future[Any]) => Future.flow(this << elem1 << elem2 <<< Future.sequence(elems.toSeq)) map cont } + + final def <<<(elems: Traversable[A]): PromiseStream[A] @cps[Future[Any]] = + shift { cont: (PromiseStream[A] => Future[Any]) => cont(this ++= elems) } + + final def <<<(elems: Future[Traversable[A]]): PromiseStream[A] @cps[Future[Any]] = + shift { cont: (PromiseStream[A] => Future[Any]) => elems map (as => cont(this ++= as)) } + } + + object Future { + + def sequence[A, M[_] <: Traversable[_]](in: M[Future[A]])(implicit cbf: CanBuildFrom[M[Future[A]], A, M[A]], executor: ExecutionContext): Future[M[A]] = + new Future(in.asInstanceOf[Traversable[Future[A]]].map((f:Future[A])=>f.x)(cbf.asInstanceOf[CanBuildFrom[Traversable[Future[A]], A, M[A]]])) + + def flow[A](body: => A @cps[Future[Any]])(implicit executor: ExecutionContext): Future[A] = reset(Future(body)).asInstanceOf[Future[A]] + + } + + def main(args: Array[String]) = { + val p = new PromiseStream[Int] + println(Future.flow(p << (Future(1), Future(2), Future(3), Future(4), Future(5)))) + } +}
\ No newline at end of file diff --git a/test/files/run/test-cpp.check b/test/files/run/test-cpp.check new file mode 100644 index 0000000000..40a976119f --- /dev/null +++ b/test/files/run/test-cpp.check @@ -0,0 +1,73 @@ +37c37 +< locals: value args, value x, value y +--- +> locals: value args +42,43d41 +< 52 CONSTANT(2) +< 52 STORE_LOCAL(value x) +45,46d42 +< 53 LOAD_LOCAL(value x) +< 53 STORE_LOCAL(value y) +49c45 +< 54 LOAD_LOCAL(value y) +--- +> 54 CONSTANT(2) +92c88 +< locals: value args, value x, value y +--- +> locals: value args, value x +101,102d96 +< 82 LOAD_LOCAL(value x) +< 82 STORE_LOCAL(value y) +105c99 +< 83 LOAD_LOCAL(value y) +--- +> 83 LOAD_LOCAL(value x) +135c129 +< locals: value args, value x, value y +--- +> locals: value args +140,141d133 +< 66 THIS(TestAliasChainDerefThis) +< 66 STORE_LOCAL(value x) +143,144d134 +< 67 LOAD_LOCAL(value x) +< 67 STORE_LOCAL(value y) +147c137 +< 68 LOAD_LOCAL(value y) +--- +> 68 THIS(Object) +176c166 +< locals: value x, value y +--- +> locals: value x +181,182d170 +< 29 LOAD_LOCAL(value x) +< 29 STORE_LOCAL(value y) +185c173 +< 30 LOAD_LOCAL(value y) +--- +> 30 LOAD_LOCAL(value x) +223,224d210 +< 97 LOAD_LOCAL(variable x) +< 97 STORE_LOCAL(variable y) +227c213 +< 98 LOAD_LOCAL(variable y) +--- +> 98 LOAD_LOCAL(variable x) +233,234d218 +< 101 LOAD_LOCAL(variable y) +< 101 STORE_LOCAL(variable x) +236c220 +< 102 LOAD_LOCAL(variable x) +--- +> 102 LOAD_LOCAL(variable y) +345c329 +< 41 THIS(TestSetterInline) +--- +> 41 THIS(Object) +347c331 +< 41 CALL_METHOD TestSetterInline._postSetHook_$eq (static-instance) +--- +> 41 STORE_FIELD variable _postSetHook (dynamic) + diff --git a/test/files/run/test-cpp.scala b/test/files/run/test-cpp.scala new file mode 100644 index 0000000000..5b3bc7b746 --- /dev/null +++ b/test/files/run/test-cpp.scala @@ -0,0 +1,104 @@ +/** + * The only change is in the decision to replace a LOAD_LOCAL(l) + * in the copy-propagation performed before ClosureElimination. + * + * In the general case, the local variable 'l' is connected through + * a alias chain with other local variables and at the end of the + * alias chain there may be a Value, call it 'v'. + * + * If 'v' is cheaper to access (it is a Deref(This) or Const(_)), then + * replace the instruction to load it from the cheaper place. + * Otherwise, we use the local variable at the end of the alias chain + * instead of 'l'. + */ + +import scala.tools.partest.IcodeTest + +object Test extends IcodeTest { + override def printIcodeAfterPhase = "dce" +} + +import scala.util.Random._ + +/** + * The example in the bug report (Issue-5321): an alias chain which store + * an Unknown. Should remove local variable 'y'. + */ +object TestBugReport { + def test(x: Int) = { + val y = x + println(y) + } +} + +/** + * The code taken from scala.tools.nsc.settings.Settings: + * After inlining of the setter is performed, there is an opportunity for + * copy-propagation to eliminate some local variables. + */ +object TestSetterInline { + private var _postSetHook: this.type => Unit = (x: this.type) => () + def withPostSetHook(f: this.type => Unit): this.type = { _postSetHook = f ; this } +} + + +/** + * The access of the local variable 'y' should be replaced by the + * constant. + */ +object TestAliasChainConstat { + + def main(args: Array[String]): Unit = { + val x = 2 + val y = x + println(y) + } +} + +/** + * At the end of the alias chain we have a reference to 'this'. + * The local variables should be all discarded and replace by a + * direct reference to this + */ +class TestAliasChainDerefThis { + + def main(args: Array[String]): Unit = { + val x = this + val y = x + println(y) + } +} + +/** + * At the end of the alias chain, there is the value of a field. + * The use of variable 'y' should be replaced by 'x', not by an access + * to the field 'f' since it is more costly. + */ +object TestAliasChainDerefField { + def f = nextInt + + def main(args: Array[String]): Unit = { + val x = f + val y = x + println(y) + } +} + + +/** + * The first time 'println' is called, 'x' is replaced by 'y' + * and the second time, 'y' is replaced by 'x'. But none of them + * can be removed. + */ +object TestDifferentBindings { + + def main(args: Array[String]): Unit = { + var x = nextInt + var y = x + println(y) + + y = nextInt + x = y + println(x) + } +} |