diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2014-05-18 12:01:17 +0200 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2014-05-19 22:27:50 +0200 |
commit | ccc5eef051c8588c1fe4029e832d5b6387976aa6 (patch) | |
tree | 8bb35044f3f935e82c6db343252f970a3cf8953c /src | |
parent | 5b1d43d8ffda4b5881df7ce6634a7adebbb02f21 (diff) | |
download | scala-ccc5eef051c8588c1fe4029e832d5b6387976aa6.tar.gz scala-ccc5eef051c8588c1fe4029e832d5b6387976aa6.tar.bz2 scala-ccc5eef051c8588c1fe4029e832d5b6387976aa6.zip |
SI-8601 Avoid over-eager optimization of LOAD_FIELD
It can NPE or trigger static class initilization, we can't elimiate
it without changing semantics.
To make sure we don't thwart closure elimination, I've allowed DCE
to eliminate a non-static LOAD_FIELD of a member of a closure class.
It would be more general to track nullity of the reciever (e.g, `this`
or `new Foo` cannot be null), but that would require more
infrastructure in this phase.
I've added a test for closure inlining based on a a suggestion by
@dragos. This actually passes if we remove the (LOAD_FIELD, DROP)
peephole optimization for `closelim` altogether. But I chose to
adapt that optimization (only allow it for non-static, closure
fields), rather then remove it alogether, in the interests of
treading lightly.
Diffstat (limited to 'src')
-rw-r--r-- | src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala | 7 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala | 5 |
2 files changed, 7 insertions, 5 deletions
diff --git a/src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala b/src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala index c49f23852f..a866173a88 100644 --- a/src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala +++ b/src/compiler/scala/tools/nsc/backend/opt/ClosureElimination.scala @@ -56,11 +56,8 @@ abstract class ClosureElimination extends SubComponent { case (BOX(t1), UNBOX(t2)) if (t1 == t2) => Some(Nil) - case (LOAD_FIELD(sym, isStatic), DROP(_)) if !sym.hasAnnotation(definitions.VolatileAttr) => - if (isStatic) - Some(Nil) - else - Some(DROP(REFERENCE(definitions.ObjectClass)) :: Nil) + case (LOAD_FIELD(sym, /* isStatic */false), DROP(_)) if !sym.hasAnnotation(definitions.VolatileAttr) && inliner.isClosureClass(sym.owner) => + Some(DROP(REFERENCE(definitions.ObjectClass)) :: Nil) case _ => None } diff --git a/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala b/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala index 3c983e6fdf..18bf63e237 100644 --- a/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala +++ b/src/compiler/scala/tools/nsc/backend/opt/DeadCodeElimination.scala @@ -172,6 +172,11 @@ abstract class DeadCodeElimination extends SubComponent { LOAD_EXCEPTION(_) | SWITCH(_, _) | MONITOR_ENTER() | MONITOR_EXIT() | CHECK_CAST(_) => moveToWorkList() + case LOAD_FIELD(sym, isStatic) if isStatic || !inliner.isClosureClass(sym.owner) => + // static load may trigger static initization. + // non-static load can throw NPE (but we know closure fields can't be accessed via a + // null reference. + moveToWorkList() case CALL_METHOD(m1, _) if isSideEffecting(m1) => moveToWorkList() |