Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opt: Get rid of the LiftTry phase; instead handle things in the back-end. #18619

Merged
merged 2 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ trait BCodeSkelBuilder extends BCodeHelpers {
private var stack = new Array[BType](32)
private var size = 0

def isEmpty: Boolean = size == 0

def push(btype: BType): Unit =
if size == stack.length then
stack = java.util.Arrays.copyOf(stack, stack.length * 2)
Expand Down Expand Up @@ -81,6 +83,16 @@ trait BCodeSkelBuilder extends BCodeHelpers {

def clear(): Unit =
size = 0

def acquireFullStack(): IArray[BType] =
val res = IArray.unsafeFromArray(stack.slice(0, size))
size = 0
res

def restoreFullStack(fullStack: IArray[BType]): Unit =
assert(size == 0 && stack.length >= fullStack.length)
fullStack.copyToArray(stack)
size = fullStack.length
end BTypesStack

object BTypesStack:
Expand Down
58 changes: 57 additions & 1 deletion compiler/src/dotty/tools/backend/jvm/BCodeSyncAndTry.scala
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ trait BCodeSyncAndTry extends BCodeBodyBuilder {

/*
* Emitting try-catch is easy, emitting try-catch-finally not quite so.
*
* For a try-catch, the only thing we need to care about is to stash the stack away
* in local variables and load them back in afterwards, in case the incoming stack
* is not empty.
*
* A finally-block (which always has type Unit, thus leaving the operand stack unchanged)
* affects control-transfer from protected regions, as follows:
*
Expand Down Expand Up @@ -190,7 +195,7 @@ trait BCodeSyncAndTry extends BCodeBodyBuilder {
}
}

// ------ (0) locals used later ------
// ------ locals used later ------

/*
* `postHandlers` is a program point denoting:
Expand All @@ -203,6 +208,13 @@ trait BCodeSyncAndTry extends BCodeBodyBuilder {
*/
val postHandlers = new asm.Label

// stack stash
val needStackStash = !stack.isEmpty && !caseHandlers.isEmpty
val acquiredStack = if needStackStash then stack.acquireFullStack() else null
val stashLocals =
if acquiredStack == null then null
else acquiredStack.uncheckedNN.filter(_ != UNIT).map(btpe => locals.makeTempLocal(btpe))
lrytz marked this conversation as resolved.
Show resolved Hide resolved

val hasFinally = (finalizer != tpd.EmptyTree)

/*
Expand All @@ -222,6 +234,17 @@ trait BCodeSyncAndTry extends BCodeBodyBuilder {
*/
val finCleanup = if (hasFinally) new asm.Label else null

/* ------ (0) Stash the stack into local variables, if necessary.
* From top of the stack down to the bottom.
* ------
*/

if stashLocals != null then
val stashLocalsNN = stashLocals.uncheckedNN // why is this necessary?
for i <- (stashLocalsNN.length - 1) to 0 by -1 do
val local = stashLocalsNN(i)
bc.store(local.idx, local.tk)

/* ------ (1) try-block, protected by:
* (1.a) the EHs due to case-clauses, emitted in (2),
* (1.b) the EH due to finally-clause, emitted in (3.A)
Expand Down Expand Up @@ -367,6 +390,39 @@ trait BCodeSyncAndTry extends BCodeBodyBuilder {
emitFinalizer(finalizer, tmp, isDuplicate = false) // the only invocation of emitFinalizer with `isDuplicate == false`
}

/* ------ (5) Unstash the stack, if it was stashed before.
* From bottom of the stack to the top.
* If there is a non-UNIT result, we need to temporarily store
* that one in a local variable while we unstash.
* ------
*/

if stashLocals != null then
val stashLocalsNN = stashLocals.uncheckedNN // why is this necessary?

val resultLoc =
if kind == UNIT then null
else if tmp != null then locals(tmp) // reuse the same local
else locals.makeTempLocal(kind)
if resultLoc != null then
bc.store(resultLoc.idx, kind)

for i <- 0 until stashLocalsNN.size do
val local = stashLocalsNN(i)
bc.load(local.idx, local.tk)
if local.tk.isRef then
bc.emit(asm.Opcodes.ACONST_NULL)
bc.store(local.idx, local.tk)

stack.restoreFullStack(acquiredStack.nn)

if resultLoc != null then
bc.load(resultLoc.idx, kind)
if kind.isRef then
bc.emit(asm.Opcodes.ACONST_NULL)
bc.store(resultLoc.idx, kind)
end if // stashLocals != null

kind
} // end of genLoadTry()

Expand Down
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ class Compiler {
new Getters, // Replace non-private vals and vars with getter defs (fields are added later)
new SpecializeFunctions, // Specialized Function{0,1,2} by replacing super with specialized super
new SpecializeTuples, // Specializes Tuples by replacing tuple construction and selection trees
new LiftTry, // Put try expressions that might execute on non-empty stacks into their own methods
new CollectNullableFields, // Collect fields that can be nulled out after use in lazy initialization
new ElimOuterSelect, // Expand outer selections
new ResolveSuper, // Implement super accessors
Expand Down
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/core/NameKinds.scala
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,6 @@ object NameKinds {
val TailTempName: UniqueNameKind = new UniqueNameKind("$tmp")
val ExceptionBinderName: UniqueNameKind = new UniqueNameKind("ex")
val SkolemName: UniqueNameKind = new UniqueNameKind("?")
val LiftedTreeName: UniqueNameKind = new UniqueNameKind("liftedTree")
val SuperArgName: UniqueNameKind = new UniqueNameKind("$superArg$")
val DocArtifactName: UniqueNameKind = new UniqueNameKind("$doc")
val UniqueInlineName: UniqueNameKind = new UniqueNameKind("$i")
Expand Down
38 changes: 4 additions & 34 deletions compiler/src/dotty/tools/dotc/transform/CapturedVars.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ class CapturedVars extends MiniPhase with IdentityDenotTransformer:

override def description: String = CapturedVars.description

override def runsAfterGroupsOf: Set[String] = Set(LiftTry.name)
// lifting tries changes what variables are considered to be captured

private[this] var Captured: Store.Location[util.ReadOnlySet[Symbol]] = _
private def captured(using Context) = ctx.store(Captured)

Expand Down Expand Up @@ -131,42 +128,15 @@ class CapturedVars extends MiniPhase with IdentityDenotTransformer:
*
* intRef.elem = expr
*
* rewrite using a temporary var to
*
* val ev$n = expr
* intRef.elem = ev$n
*
* That way, we avoid the problem that `expr` might contain a `try` that would
* run on a non-empty stack (which is illegal under JVM rules). Note that LiftTry
* has already run before, so such `try`s would not be eliminated.
*
* If the ref type lhs is followed by a cast (can be an artifact of nested translation),
* drop the cast.
*
* If the ref type is `ObjectRef` or `VolatileObjectRef`, immediately assign `null`
* to the temporary to make the underlying target of the reference available for
* garbage collection. Nullification is omitted if the `expr` is already `null`.
*
* var ev$n: RHS = expr
* objRef.elem = ev$n
* ev$n = null.asInstanceOf[RHS]
* the lhs can be followed by a cast as an artifact of nested translation.
* In that case, drop the cast.
*/
override def transformAssign(tree: Assign)(using Context): Tree =
def absolved: Boolean = tree.rhs match
case Literal(Constant(null)) | Typed(Literal(Constant(null)), _) => true
case _ => false
def recur(lhs: Tree): Tree = lhs match
tree.lhs match
case TypeApply(Select(qual@Select(_, nme.elem), nme.asInstanceOf_), _) =>
recur(qual)
case Select(_, nme.elem) if refInfo.boxedRefClasses.contains(lhs.symbol.maybeOwner) =>
val tempDef = transformFollowing(SyntheticValDef(TempResultName.fresh(), tree.rhs, flags = Mutable))
val update = cpy.Assign(tree)(lhs, ref(tempDef.symbol))
def reset = cpy.Assign(tree)(ref(tempDef.symbol), nullLiteral.cast(tempDef.symbol.info))
val res = if refInfo.objectRefClasses(lhs.symbol.maybeOwner) && !absolved then reset else unitLiteral
transformFollowing(Block(tempDef :: update :: Nil, res))
cpy.Assign(tree)(qual, tree.rhs)
case _ =>
tree
recur(tree.lhs)

object CapturedVars:
val name: String = "capturedVars"
Expand Down
88 changes: 0 additions & 88 deletions compiler/src/dotty/tools/dotc/transform/LiftTry.scala

This file was deleted.

1 change: 0 additions & 1 deletion docs/_docs/internals/overall-structure.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ phases. The current list of phases is specified in class [Compiler] as follows:
new InterceptedMethods, // Special handling of `==`, `|=`, `getClass` methods
new Getters, // Replace non-private vals and vars with getter defs (fields are added later)
new SpecializeFunctions, // Specialized Function{0,1,2} by replacing super with specialized super
new LiftTry, // Put try expressions that might execute on non-empty stacks into their own methods
new CollectNullableFields, // Collect fields that can be nulled out after use in lazy initialization
new ElimOuterSelect, // Expand outer selections
new ResolveSuper, // Implement super accessors
Expand Down
1 change: 0 additions & 1 deletion tests/neg/t1672b.scala
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ object Test1772B {
}
}

// the `liftedTree` local method will prevent a tail call here.
@tailrec
def bar(i : Int) : Int = {
if (i == 0) 0
Expand Down
2 changes: 1 addition & 1 deletion tests/run/i1692.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class LazyNullable(a: => Int) {
lazy val l4Inf = eInf

private[this] val i = "I"
// null out i even though the try ends up lifted, because the LazyVals phase runs before the LiftTry phase
// null out i even though the try needs stack stashing
lazy val l5 = try i catch { case e: Exception => () }
}

Expand Down
2 changes: 1 addition & 1 deletion tests/run/i1692b.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class LazyNullable(a: => Int) {
@threadUnsafe lazy val l4Inf = try eInf finally () // null out e, since private[this] is inferred

private[this] val i = "I"
// null out i even though the try ends up lifted, because the LazyVals phase runs before the LiftTry phase
// null out i even though the try needs stack stashing
@threadUnsafe lazy val l5 = try i catch { case e: Exception => () }
}

Expand Down
2 changes: 0 additions & 2 deletions tests/run/i4866.check

This file was deleted.

21 changes: 0 additions & 21 deletions tests/run/i4866.scala

This file was deleted.

Loading