Do not ignore copy operations while finding source instructions

in redundant locals elimination.

Since IgnoringCopySourceInterpreter ignores ALOADs and ASTOREs,
the source instruction of ALOAD K in {ALOAD N, ASTORE K, ALOAD K}
sequence is not ASTORE K. In this case we cannot simply replace K
with N, since there can be multiple {ALOAD N, ASTORE K} sequences
in separate branches. After replacement we get different stack
frames.
This change resolves this.

However, in ReturnUnitMethodTransformer we want to ignore copies
of the same GETSTATIC kotlin/Unit.INSTANCE, since we do not mess
with local variables and just replace ASTORE with ARETURN to help
tail-call optimization.
 #KT-23373: Fixed
This commit is contained in:
Ilmir Usmanov
2018-03-21 23:52:57 +03:00
parent 15b46cdda9
commit 481dbee96a
7 changed files with 114 additions and 5 deletions
@@ -122,7 +122,7 @@ class RedundantLocalsEliminationMethodTransformer : MethodTransformer() {
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()
val sources = findSourceInstructions(internalClassName, methodNode, listOf(succ), ignoreCopy = false).values.flatten()
if (sources.size != 1) continue
res[insn] = succ
}
@@ -182,7 +182,7 @@ class RedundantLocalsEliminationMethodTransformer : MethodTransformer() {
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()
val sources = findSourceInstructions(internalClassName, methodNode, listOf(succ), ignoreCopy = false).values.flatten()
if (sources.size > 1) continue
res[insn] = succ
}
@@ -28,6 +28,7 @@ import org.jetbrains.kotlin.utils.keysToMap
import org.jetbrains.kotlin.utils.sure
import org.jetbrains.org.objectweb.asm.Opcodes
import org.jetbrains.org.objectweb.asm.tree.*
import org.jetbrains.org.objectweb.asm.tree.analysis.SourceInterpreter
/*
* Replace POP with ARETURN iff
@@ -47,7 +48,7 @@ object ReturnUnitMethodTransformer : MethodTransformer() {
val pops = methodNode.instructions.asSequence().filter { it.opcode == Opcodes.POP }.toList()
val popSuccessors = findSuccessors(methodNode, pops)
val sourceInsns = findSourceInstructions(internalClassName, methodNode, pops)
val sourceInsns = findSourceInstructions(internalClassName, methodNode, pops, ignoreCopy = true)
val safePops = filterOutUnsafes(popSuccessors, units, sourceInsns)
// Replace POP with ARETURN for tail call optimization
@@ -122,9 +123,14 @@ object ReturnUnitMethodTransformer : MethodTransformer() {
internal fun findSourceInstructions(
internalClassName: String,
methodNode: MethodNode,
insns: Collection<AbstractInsnNode>
insns: Collection<AbstractInsnNode>,
ignoreCopy: Boolean
): Map<AbstractInsnNode, Collection<AbstractInsnNode>> {
val frames = MethodTransformer.analyze(internalClassName, methodNode, IgnoringCopyOperationSourceInterpreter())
val frames = MethodTransformer.analyze(
internalClassName,
methodNode,
if (ignoreCopy) IgnoringCopyOperationSourceInterpreter() else SourceInterpreter()
)
return insns.keysToMap {
val index = methodNode.instructions.indexOf(it)
if (isUnreachable(index, frames)) return@keysToMap emptySet<AbstractInsnNode>()