Remove redundant locals

Do not transform already transformed suspend lambdas
Ignore duplicates of continuation in local variable table during redundant locals elimination.
This commit is contained in:
Ilmir Usmanov
2018-02-27 19:08:37 +03:00
parent 81f3e39f29
commit 826d667398
28 changed files with 785 additions and 113 deletions
@@ -1103,8 +1103,11 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
if (closure.isSuspend()) {
// resultContinuation
if (closure.isSuspendLambda()) {
// When inlining crossinline lambda, the ACONST_NULL is never popped.
// Thus, do not generate it. Otherwise, it leads to VerifyError on run-time.
boolean isCrossinlineLambda = (callGenerator instanceof InlineCodegen<?>) &&
Objects.requireNonNull(((InlineCodegen) callGenerator).getActiveLambda()).isCrossInline;
Objects.requireNonNull(((InlineCodegen) callGenerator).getActiveLambda(),
"no active lambda found").isCrossInline;
if (!isCrossinlineLambda) {
v.aconst(null);
}
@@ -70,6 +70,7 @@ class CodegenAnnotatingVisitor extends KtVisitorVoid {
private final Stack<ClassDescriptor> classStack = new Stack<>();
private final Stack<String> nameStack = new Stack<>();
private final Stack<FunctionDescriptor> functionsStack = new Stack<>();
private final Set<ClassDescriptor> uninitializedClasses = new HashSet<>();
private final BindingTrace bindingTrace;
@@ -317,49 +318,16 @@ class CodegenAnnotatingVisitor extends KtVisitorVoid {
);
closure.setSuspend(true);
closure.setSuspendLambda();
if (capturesCrossinlineSuspendLambda(functionLiteral)) {
bindingTrace.record(
CodegenBinding.CAPTURES_CROSSINLINE_SUSPEND_LAMBDA,
functionDescriptor,
true
);
}
}
functionsStack.push(functionDescriptor);
super.visitLambdaExpression(lambdaExpression);
functionsStack.pop();
nameStack.pop();
classStack.pop();
}
// If inner lambda captures crossinline suspend lambda, it is unsafe to generate state machine for it.
private boolean capturesCrossinlineSuspendLambda(@NotNull PsiElement psiNode) {
PsiElement[] children = psiNode.getChildren();
boolean res = false;
for (PsiElement child : children) {
if (child instanceof KtCallElement) {
KtExpression callee = ((KtCallElement) child).getCalleeExpression();
Call call = bindingContext.get(CALL, callee);
ResolvedCall<?> resolvedCall = bindingContext.get(RESOLVED_CALL, call);
if (resolvedCall instanceof VariableAsFunctionResolvedCall) {
VariableAsFunctionResolvedCall variableAsFunction = (VariableAsFunctionResolvedCall) resolvedCall;
VariableDescriptor variableDescriptor = variableAsFunction.getVariableCall().getResultingDescriptor();
CallableDescriptor callableDescriptor = ((ResolvedCall) variableAsFunction).getResultingDescriptor();
if (variableDescriptor instanceof ValueParameterDescriptor &&
((ValueParameterDescriptor) variableDescriptor).isCrossinline() &&
callableDescriptor instanceof FunctionDescriptor &&
((FunctionDescriptor) callableDescriptor).isSuspend()) {
FunctionDescriptor enclosingDescriptor =
bindingContext.get(ENCLOSING_SUSPEND_FUNCTION_FOR_SUSPEND_FUNCTION_CALL, resolvedCall.getCall());
assert(enclosingDescriptor != null);
return true;
}
}
}
res = res || capturesCrossinlineSuspendLambda(child);
}
return res;
}
@Override
public void visitCallableReferenceExpression(@NotNull KtCallableReferenceExpression expression) {
ResolvedCall<?> referencedFunction = CallUtilKt.getResolvedCall(expression.getCallableReference(), bindingContext);
@@ -573,17 +541,11 @@ class CodegenAnnotatingVisitor extends KtVisitorVoid {
recordClassForFunction(function, functionDescriptor, name, functionDescriptor);
MutableClosure closure = recordClosure(classDescriptor, name);
closure.setSuspend(true);
if (capturesCrossinlineSuspendLambda(function)) {
bindingTrace.record(
CodegenBinding.CAPTURES_CROSSINLINE_SUSPEND_LAMBDA,
functionDescriptor,
true
);
}
functionsStack.push(functionDescriptor);
super.visitNamedFunction(function);
functionsStack.pop();
if (nameForClassOrPackageMember != null) {
nameStack.pop();
}
@@ -593,7 +555,9 @@ class CodegenAnnotatingVisitor extends KtVisitorVoid {
if (nameForClassOrPackageMember != null) {
nameStack.push(nameForClassOrPackageMember);
functionsStack.push(functionDescriptor);
super.visitNamedFunction(function);
functionsStack.pop();
nameStack.pop();
}
else {
@@ -602,8 +566,10 @@ class CodegenAnnotatingVisitor extends KtVisitorVoid {
recordClosure(classDescriptor, name);
classStack.push(classDescriptor);
functionsStack.push(functionDescriptor);
nameStack.push(name);
super.visitNamedFunction(function);
functionsStack.pop();
nameStack.pop();
classStack.pop();
}
@@ -631,6 +597,47 @@ class CodegenAnnotatingVisitor extends KtVisitorVoid {
public void visitCallExpression(@NotNull KtCallExpression expression) {
super.visitCallExpression(expression);
checkSamCall(expression);
checkCrossinlineSuspendCall(expression);
}
private void checkCrossinlineSuspendCall(@NotNull KtCallExpression expression) {
KtExpression callee = expression.getCalleeExpression();
Call call = bindingContext.get(CALL, callee);
ResolvedCall<?> resolvedCall = bindingContext.get(RESOLVED_CALL, call);
if (resolvedCall instanceof VariableAsFunctionResolvedCall) {
VariableAsFunctionResolvedCall variableAsFunction = (VariableAsFunctionResolvedCall) resolvedCall;
VariableDescriptor variableDescriptor = variableAsFunction.getVariableCall().getResultingDescriptor();
CallableDescriptor callableDescriptor = ((ResolvedCall) variableAsFunction).getResultingDescriptor();
if (variableDescriptor instanceof ValueParameterDescriptor &&
((ValueParameterDescriptor) variableDescriptor).isCrossinline() &&
callableDescriptor instanceof FunctionDescriptor &&
((FunctionDescriptor) callableDescriptor).isSuspend()) {
FunctionDescriptor enclosingDescriptor =
bindingContext.get(ENCLOSING_SUSPEND_FUNCTION_FOR_SUSPEND_FUNCTION_CALL, resolvedCall.getCall());
if (enclosingDescriptor == null) return;
DeclarationDescriptor functionWithCrossinlineParameter = variableDescriptor.getContainingDeclaration();
for (int i = functionsStack.size() - 1; i >= 0; i--) {
if (functionsStack.get(i).isSuspend()) {
Boolean alreadyPutValue = bindingTrace.getBindingContext()
.get(CodegenBinding.CAPTURES_CROSSINLINE_SUSPEND_LAMBDA, functionsStack.get(i));
if (alreadyPutValue != null && alreadyPutValue) {
return;
}
bindingTrace.record(
CodegenBinding.CAPTURES_CROSSINLINE_SUSPEND_LAMBDA,
functionsStack.get(i),
true
);
}
if (functionsStack.get(i) == functionWithCrossinlineParameter) {
return;
}
}
}
}
}
private void checkSamCall(@NotNull KtCallElement expression) {
@@ -67,20 +67,24 @@ class CoroutineTransformerMethodVisitor(
override fun performTransformations(methodNode: MethodNode) {
removeFakeContinuationConstructorCall(methodNode)
replaceFakeContinuationsWithRealOnes(
methodNode,
if (isForNamedFunction) getLastParameterIndex(methodNode.desc, methodNode.access) else 0
)
FixStackMethodTransformer().transform(containingClassInternalName, methodNode)
RedundantLocalsEliminationMethodTransformer().transform(containingClassInternalName, methodNode)
updateMaxStack(methodNode)
val suspensionPoints = collectSuspensionPoints(methodNode)
// First instruction in the method node may change in case of named function
val actualCoroutineStart = methodNode.instructions.first
FixStackMethodTransformer().transform(containingClassInternalName, methodNode)
if (isForNamedFunction) {
ReturnUnitMethodTransformer.transform(containingClassInternalName, methodNode)
if (allSuspensionPointsAreTailCalls(containingClassInternalName, methodNode, suspensionPoints)) {
continuationIndex = getLastParameterIndex(methodNode.desc, methodNode.access)
replaceFakeContinuationsWithRealOnes(methodNode, continuationIndex)
dropSuspensionMarkers(methodNode, suspensionPoints)
return
}
@@ -101,8 +105,6 @@ class CoroutineTransformerMethodVisitor(
// Actual max stack might be increased during the previous phases
updateMaxStack(methodNode)
replaceFakeContinuationsWithRealOnes(methodNode, continuationIndex)
// Remove unreachable suspension points
// If we don't do this, then relevant frames will not be analyzed, that is unexpected from point of view of next steps (e.g. variable spilling)
removeUnreachableSuspensionPointsAndExitPoints(methodNode, suspensionPoints)
@@ -820,7 +822,7 @@ private fun AbstractInsnNode?.isInvisibleInDebugVarInsn(methodNode: MethodNode):
private val SAFE_OPCODES =
((Opcodes.DUP..Opcodes.DUP2_X2) + Opcodes.NOP + Opcodes.POP + Opcodes.POP2 + (Opcodes.IFEQ..Opcodes.GOTO)).toSet()
internal fun replaceFakeContinuationsWithRealOnes(methodNode: MethodNode, continuationIndex: Int) {
private fun replaceFakeContinuationsWithRealOnes(methodNode: MethodNode, continuationIndex: Int) {
val fakeContinuations = methodNode.instructions.asSequence().filter(::isFakeContinuationMarker).toList()
for (fakeContinuation in fakeContinuations) {
methodNode.instructions.removeAll(listOf(fakeContinuation.previous.previous, fakeContinuation.previous))
@@ -0,0 +1,218 @@
/*
* Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
* that can be found in the license/LICENSE.txt file.
*/
package org.jetbrains.kotlin.codegen.coroutines
import org.jetbrains.kotlin.codegen.optimization.boxing.isUnitInstance
import org.jetbrains.kotlin.codegen.optimization.common.ControlFlowGraph
import org.jetbrains.kotlin.codegen.optimization.common.asSequence
import org.jetbrains.kotlin.codegen.optimization.common.isMeaningful
import org.jetbrains.kotlin.codegen.optimization.common.removeAll
import org.jetbrains.kotlin.codegen.optimization.transformer.MethodTransformer
import org.jetbrains.org.objectweb.asm.Opcodes
import org.jetbrains.org.objectweb.asm.tree.*
// Inliner emits a lot of locals during inlining.
// Remove all of them since these locals are
// 1) going to be spilled into continuation object
// 2) breaking tail-call elimination
class RedundantLocalsEliminationMethodTransformer : MethodTransformer() {
lateinit var internalClassName: String
override fun transform(internalClassName: String, methodNode: MethodNode) {
this.internalClassName = internalClassName
do {
var changed = false
changed = simpleRemove(methodNode) || changed
changed = removeWithReplacement(methodNode) || changed
changed = removeAloadCheckcastContinuationAstore(methodNode) || changed
} while (changed)
}
// Replace
// GETSTATIC kotlin/Unit.INSTANCE
// ASTORE N
// ...
// ALOAD N
// with
// ...
// GETSTATIC kotlin/Unit.INSTANCE
// or
// ACONST_NULL
// ASTORE N
// ...
// ALOAD N
// with
// ...
// ACONST_NULL
// or
// ALOAD K
// ASTORE N
// ...
// ALOAD N
// with
// ...
// ALOAD K
//
// But do not remove several at a time, since the same local (for example, ALOAD 0) might be loaded and stored multiple times in
// sequence, like
// ALOAD 0
// ASTORE 1
// ALOAD 1
// ASTORE 2
// ALOAD 3
// Here, it is unsafe to replace ALOAD 3 with ALOAD 1, and then already removed ALOAD 1 with ALOAD 0.
private fun removeWithReplacement(
methodNode: MethodNode
): Boolean {
val insns = findSafeAstorePredecessors(methodNode, ignoreLocalVariableTable = false) {
it.isUnitInstance() || it.opcode == Opcodes.ACONST_NULL || it.opcode == Opcodes.ALOAD
}
insns.asIterable().firstOrNull { (pred, astore) ->
val index = astore.localIndex()
methodNode.instructions.removeAll(listOf(pred, astore))
methodNode.instructions.asSequence()
.filter { it.opcode == Opcodes.ALOAD && it.localIndex() == index }
.toList()
.forEach { methodNode.instructions.set(it, pred.clone()) }
return true
}
return false
}
private fun AbstractInsnNode.clone() = when (this) {
is FieldInsnNode -> FieldInsnNode(opcode, owner, name, desc)
is VarInsnNode -> VarInsnNode(opcode, `var`)
is InsnNode -> InsnNode(opcode)
is TypeInsnNode -> TypeInsnNode(opcode, desc)
else -> error("clone of $this is not implemented yet")
}
// Remove
// ALOAD N
// POP
// or
// ACONST_NULL
// POP
// or
// GETSTATIC kotlin/Unit.INSTANCE
// POP
private fun simpleRemove(methodNode: MethodNode): Boolean {
val insns =
findPopPredecessors(methodNode) { it.isUnitInstance() || it.opcode == Opcodes.ACONST_NULL || it.opcode == Opcodes.ALOAD }
for ((pred, pop) in insns) {
methodNode.instructions.removeAll(listOf(pred, pop))
}
return insns.isNotEmpty()
}
private fun findPopPredecessors(
methodNode: MethodNode,
predicate: (AbstractInsnNode) -> Boolean
): Map<AbstractInsnNode, AbstractInsnNode> {
val insns = methodNode.instructions.asSequence().filter { predicate(it) }.toList()
val cfg = ControlFlowGraph.build(methodNode)
val res = hashMapOf<AbstractInsnNode, AbstractInsnNode>()
for (insn in insns) {
val succ = findImmediateSuccessors(insn, cfg, methodNode).singleOrNull() ?: continue
if (succ.opcode != Opcodes.POP) continue
if (insn.opcode == Opcodes.ALOAD && methodNode.localVariables.firstOrNull { it.index == insn.localIndex() } != null) continue
val sources = findSourceInstructions(internalClassName, methodNode, listOf(succ)).values.flatten()
if (sources.size != 1) continue
res[insn] = succ
}
return res
}
// Replace
// ALOAD K
// CHECKCAST Continuation
// ASTORE N
// ...
// ALOAD N
// with
// ...
// ALOAD K
// CHECKCAST Continuation
private fun removeAloadCheckcastContinuationAstore(methodNode: MethodNode): Boolean {
// Here we ignore the duplicates of continuation in local variable table,
// Since it increases performance greatly.
val insns = findSafeAstorePredecessors(methodNode, ignoreLocalVariableTable = true) {
it.opcode == Opcodes.CHECKCAST &&
(it as TypeInsnNode).desc == CONTINUATION_ASM_TYPE.internalName &&
it.previous?.opcode == Opcodes.ALOAD
}
for ((checkcast, astore) in insns) {
val aload = checkcast.previous
val index = astore.localIndex()
methodNode.instructions.removeAll(listOf(aload, checkcast, astore))
methodNode.instructions.asSequence()
.filter { it.opcode == Opcodes.ALOAD && it.localIndex() == index }
.toList()
.forEach {
methodNode.instructions.insertBefore(it, aload.clone())
methodNode.instructions.set(it, checkcast.clone())
}
}
return insns.isNotEmpty()
}
private fun findSafeAstorePredecessors(
methodNode: MethodNode,
ignoreLocalVariableTable: Boolean,
predicate: (AbstractInsnNode) -> Boolean
): Map<AbstractInsnNode, AbstractInsnNode> {
val insns = methodNode.instructions.asSequence().filter { predicate(it) }.toList()
val cfg = ControlFlowGraph.build(methodNode)
val res = hashMapOf<AbstractInsnNode, AbstractInsnNode>()
for (insn in insns) {
val succ = findImmediateSuccessors(insn, cfg, methodNode).singleOrNull() ?: continue
if (succ.opcode != Opcodes.ASTORE) continue
if (methodNode.instructions.asSequence().count {
it.opcode == Opcodes.ASTORE && it.localIndex() == succ.localIndex()
} != 1) continue
if (!ignoreLocalVariableTable && methodNode.localVariables.firstOrNull { it.index == succ.localIndex() } != null) continue
val sources = findSourceInstructions(internalClassName, methodNode, listOf(succ)).values.flatten()
if (sources.size > 1) continue
res[insn] = succ
}
return res
}
// Find all meaningful successors of insn
private fun findImmediateSuccessors(
insn: AbstractInsnNode,
cfg: ControlFlowGraph,
methodNode: MethodNode
): Collection<AbstractInsnNode> {
val visited = hashSetOf<AbstractInsnNode>()
fun dfs(current: AbstractInsnNode): Collection<AbstractInsnNode> {
if (!visited.add(current)) return emptySet()
return cfg.getSuccessorsIndices(current).flatMap {
val succ = methodNode.instructions[it]
if (!succ.isMeaningful || succ is JumpInsnNode || succ.opcode == Opcodes.NOP) dfs(succ)
else setOf(succ)
}
}
return dfs(insn)
}
private fun AbstractInsnNode.localIndex(): Int {
assert(this is VarInsnNode)
return (this as VarInsnNode).`var`
}
}
@@ -106,19 +106,6 @@ object ReturnUnitMethodTransformer : MethodTransformer() {
private fun isSuspendingCallReturningUnit(node: AbstractInsnNode): Boolean =
node.safeAs<MethodInsnNode>()?.next?.next?.let(::isReturnsUnitMarker) == true
private fun findSourceInstructions(
internalClassName: String,
methodNode: MethodNode,
pops: Collection<AbstractInsnNode>
): Map<AbstractInsnNode, Collection<AbstractInsnNode>> {
val frames = analyze(internalClassName, methodNode, IgnoringCopyOperationSourceInterpreter())
return pops.keysToMap {
val index = methodNode.instructions.indexOf(it)
if (isUnreachable(index, frames)) return@keysToMap emptySet<AbstractInsnNode>()
frames[index].getStack(0).insns
}
}
// Find { GETSTATIC kotlin/Unit.INSTANCE, ARETURN } sequences
// Result is list of GETSTATIC kotlin/Unit.INSTANCE instructions
private fun findReturnUnitSequences(methodNode: MethodNode): Collection<AbstractInsnNode> =
@@ -132,3 +119,15 @@ object ReturnUnitMethodTransformer : MethodTransformer() {
}
}
internal fun findSourceInstructions(
internalClassName: String,
methodNode: MethodNode,
insns: Collection<AbstractInsnNode>
): Map<AbstractInsnNode, Collection<AbstractInsnNode>> {
val frames = MethodTransformer.analyze(internalClassName, methodNode, IgnoringCopyOperationSourceInterpreter())
return insns.keysToMap {
val index = methodNode.instructions.indexOf(it)
if (isUnreachable(index, frames)) return@keysToMap emptySet<AbstractInsnNode>()
frames[index].getStack(0).insns
}
}
@@ -177,19 +177,14 @@ private fun NewResolvedCallImpl<VariableDescriptor>.asDummyOldResolvedCall(bindi
}
fun ResolvedCall<*>.isSuspendNoInlineCall(codegen: ExpressionCodegen): Boolean {
var isCrossinline = false
var isInlineLambda = false
if (this is VariableAsFunctionResolvedCall) {
variableCall.resultingDescriptor.safeAs<ValueParameterDescriptor>()?.let {
isCrossinline = it.isCrossinline
isInlineLambda = !isCrossinline && !it.isNoinline && codegen.context.functionDescriptor.isInline
}
}
return resultingDescriptor.safeAs<FunctionDescriptor>()
?.let {
val inline = it.isInline || isCrossinline || isInlineLambda
it.isSuspend && (!inline || it.isBuiltInSuspendCoroutineOrReturnInJvm() || it.isBuiltInSuspendCoroutineUninterceptedOrReturnInJvm())
} == true
val isInlineLambda = this.safeAs<VariableAsFunctionResolvedCall>()
?.variableCall?.resultingDescriptor?.safeAs<ValueParameterDescriptor>()
?.let { it.isCrossinline || (!it.isNoinline && codegen.context.functionDescriptor.isInline) } == true
val functionDescriptor = resultingDescriptor as? FunctionDescriptor ?: return false
if (!functionDescriptor.isSuspend) return false
if (functionDescriptor.isBuiltInSuspendCoroutineOrReturnInJvm() || functionDescriptor.isBuiltInSuspendCoroutineUninterceptedOrReturnInJvm()) return true
return !(functionDescriptor.isInline || isInlineLambda)
}
fun CallableDescriptor.isSuspendFunctionNotSuspensionView(): Boolean {
@@ -87,7 +87,8 @@ abstract class InlineCodegen<out T: BaseExpressionCodegen>(
protected val expressionMap = linkedMapOf<Int, LambdaInfo>()
var activeLambda: LambdaInfo? = null; protected set
var activeLambda: LambdaInfo? = null
protected set
private val defaultSourceMapper = sourceCompiler.lazySourceMapper
@@ -10,7 +10,6 @@ import org.jetbrains.kotlin.codegen.AsmUtil
import org.jetbrains.kotlin.codegen.ClosureCodegen
import org.jetbrains.kotlin.codegen.StackValue
import org.jetbrains.kotlin.codegen.coroutines.CONTINUATION_ASM_TYPE
import org.jetbrains.kotlin.codegen.coroutines.replaceFakeContinuationsWithRealOnes
import org.jetbrains.kotlin.codegen.inline.FieldRemapper.Companion.foldName
import org.jetbrains.kotlin.codegen.intrinsics.IntrinsicMethods
import org.jetbrains.kotlin.codegen.optimization.ApiVersionCallsPreprocessingMethodTransformer
@@ -25,7 +25,7 @@ class RegeneratedLambdaFieldRemapper(
originalLambdaInternalName: String,
override val newLambdaInternalName: String,
parameters: Parameters,
private val recapturedLambdas: Map<String, LambdaInfo>,
val recapturedLambdas: Map<String, LambdaInfo>,
remapper: FieldRemapper,
private val isConstructor: Boolean
) : FieldRemapper(originalLambdaInternalName, remapper, parameters) {