azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

JS/Inlining: always create temporary variables to alias expressions in following cases:

Browse Source 
* when decomposing expression that contains inline calls, alias all subexpressions
* when substituting arguments to inline call
* for value returned from inline function.

Instead, rely on dedicated optimization pass. Improve lookup of function to inline, since the old one relied on immediate optimizations. Give more hints to optimizer.

This should make inlining more stable in different hard-to-reproduce corner cases with evaluation order.
This commit is contained in:
Alexey Andreev
2016-05-04 16:48:15 +03:00
parent c11f2fe2d6
commit c08e2e8ca0
14 changed files with 129 additions and 108 deletions
Download Patch File Download Diff File Expand all files Collapse all files
js/js.inliner/src/org/jetbrains/kotlin/js/inline/ExpressionDecomposer.kt
+11 -12
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2010-2015 JetBrains s.r.o.
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -17,10 +17,10 @@
package org.jetbrains.kotlin.js.inline
import com.google.dart.compiler.backend.js.ast.*
import com.google.dart.compiler.backend.js.ast.metadata.staticRef
import com.google.dart.compiler.backend.js.ast.metadata.synthetic
import com.intellij.util.SmartList
import org.jetbrains.kotlin.js.inline.util.IdentitySet
import org.jetbrains.kotlin.js.inline.util.canHaveOwnSideEffect
import org.jetbrains.kotlin.js.translate.utils.JsAstUtils.*
import org.jetbrains.kotlin.js.translate.utils.jsAstUtils.*
@@ -62,7 +62,7 @@ internal class ExpressionDecomposer private constructor(
val decomposer = with (statement) {
val extractable = match(canBeExtractedByInliner)
val containsExtractable = withParentsOfNodes(extractable)
val nodesWithSideEffect = match { it is JsExpression && it.canHaveOwnSideEffect() }
val nodesWithSideEffect = match { it !is JsLiteral.JsValueLiteral }
val containsNodeWithSideEffect = withParentsOfNodes(nodesWithSideEffect)
ExpressionDecomposer(scope, containsExtractable, containsNodeWithSideEffect)
@@ -169,7 +169,7 @@ internal class ExpressionDecomposer private constructor(
// Must be (someThingWithSideEffect).x = arg2, because arg1 can have side effect
assert(arg1 is JsNameRef) { "Valid JavaScript left-hand side must be JsNameRef, got: $this" }
val arg1AsRef = arg1 as JsNameRef
arg1AsRef.qualifier = arg1AsRef.qualifier!!.extractToTemporary()
arg1AsRef.qualifier = arg1AsRef.qualifier?.extractToTemporary()
}
else {
arg1 = arg1.extractToTemporary()
@@ -317,22 +317,21 @@ internal class ExpressionDecomposer private constructor(
addStatement(tmp.variable)
return tmp.nameRef
}
private inner class Temporary(val value: JsExpression? = null) {
val name: JsName = scope.declareTemporary()
val variable: JsVars = newVar(name, value)
val variable: JsVars = newVar(name, value).apply {
synthetic = true
name.staticRef = value
}
val nameRef: JsExpression
get() = name.makeRef()
init {
variable.synthetic = true
}
fun assign(value: JsExpression): JsStatement {
val statement = JsExpressionStatement(assignment(nameRef, value))
statement.synthetic = true
val statement = JsExpressionStatement(assignment(nameRef, value)).apply { synthetic = true }
name.staticRef = value
return statement
}
}
js/js.inliner/src/org/jetbrains/kotlin/js/inline/FunctionInlineMutator.kt
+7 -7
View File
@@ -28,7 +28,7 @@ import org.jetbrains.kotlin.js.inline.util.rewriters.ReturnReplacingVisitor
class FunctionInlineMutator
private constructor(
private val call: JsInvocation,
private val inliningContext: InliningContext
inliningContext: InliningContext
) {
private val invokedFunction: JsFunction
private val namingContext: NamingContext
@@ -85,7 +85,9 @@ private constructor(
}
private fun applyCapturedArgs(call: JsInvocation, inner: JsFunction, outer: JsFunction) {
val namingContext = inliningContext.newNamingContext()
// We want statements that introduce temporary variables to be added immediately after applying renamings,
// so that further processing that involves detection of inlineable function had a chance to recognize them.
val namingContext = NamingContext(inner.scope) { inner.body.statements.addAll(0, it) }
val arguments = call.arguments
val parameters = outer.parameters
aliasArgumentsIfNeeded(namingContext, arguments, parameters)
@@ -98,11 +100,9 @@ private constructor(
var thisReplacement = getThisReplacement(call)
if (thisReplacement == null || thisReplacement is JsLiteral.JsThisRef) return
if (thisReplacement.needToAlias()) {
val thisName = namingContext.getFreshName(getThisAlias())
namingContext.newVar(thisName, thisReplacement)
thisReplacement = thisName.makeRef()
}
val thisName = namingContext.getFreshName(getThisAlias())
namingContext.newVar(thisName, thisReplacement)
thisReplacement = thisName.makeRef()
replaceThisReference(block, thisReplacement)
}
js/js.inliner/src/org/jetbrains/kotlin/js/inline/JsInliner.java
+10 -4
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2010-2015 JetBrains s.r.o.
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -19,6 +19,7 @@ package org.jetbrains.kotlin.js.inline;
import com.google.dart.compiler.backend.js.ast.*;
import com.google.dart.compiler.backend.js.ast.metadata.MetadataProperties;
import com.intellij.psi.PsiElement;
import kotlin.Unit;
import kotlin.jvm.functions.Function1;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -58,7 +59,6 @@ public class JsInliner extends JsVisitorWithContextImpl {
if (!(node instanceof JsInvocation)) return false;
JsInvocation call = (JsInvocation) node;
return hasToBeInlined(call);
}
};
@@ -199,7 +199,7 @@ public class JsInliner extends JsVisitorWithContextImpl {
if (currentStatement instanceof JsExpressionStatement &&
((JsExpressionStatement) currentStatement).getExpression() == call &&
(resultExpression == null || !SideEffectUtilsKt.canHaveSideEffect(resultExpression))
resultExpression == null
) {
statementContext.removeMe();
}
@@ -267,7 +267,13 @@ public class JsInliner extends JsVisitorWithContextImpl {
@Override
public NamingContext newNamingContext() {
JsScope scope = getFunctionContext().getScope();
return new NamingContext(scope, getStatementContext());
return new NamingContext(scope, new Function1<List<? extends JsStatement>, Unit>() {
@Override
public Unit invoke(List<? extends JsStatement> statements) {
getStatementContext().addPrevious(statements);
return null;
}
});
}
@NotNull
js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/ReferenceTracker.kt
+2 -2
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2010-2015 JetBrains s.r.o.
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -67,7 +67,7 @@ internal class ReferenceTracker<in Reference, RemoveCandidate : JsNode> {
return referenceFromTo.getOrPut(referrer, { IdentitySet<Reference>() })
}
private fun isKnown(ref: Reference): Boolean {
fun isKnown(ref: Reference): Boolean {
return removableCandidates.containsKey(ref)
}
js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryAssignmentElimination.kt
+1 -1
View File
@@ -69,7 +69,7 @@ internal class TemporaryAssignmentElimination(private val root: JsBlock) {
val propertyMutation = JsAstUtils.decomposeAssignment(x.expression)
if (propertyMutation != null) {
val (target, value) = propertyMutation
if (!target.canHaveSideEffect()) {
if (!target.canHaveSideEffect(namesToProcess)) {
val usage = Usage.PropertyMutation(x, target)
tryRecord(value, usage)
accept(value)
js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/removeUnusedLocalFunctionDeclarations.kt
+25 -4
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2010-2015 JetBrains s.r.o.
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -20,6 +20,7 @@ import com.google.dart.compiler.backend.js.ast.*
import com.google.dart.compiler.backend.js.ast.metadata.staticRef
import org.jetbrains.kotlin.js.inline.util.collectUsedNames
import org.jetbrains.kotlin.js.inline.util.transitiveStaticRef
/**
* Removes unused local function declarations like:
@@ -55,7 +56,7 @@ private class UnusedInstanceCollector : JsVisitorWithContextImpl() {
val references = collectUsedNames(x)
references.filterNotNull()
.forEach { tracker.addRemovableReference(name, it) }
.forEach { tracker.addRemovableReference(name, it) }
return false
}
@@ -73,8 +74,28 @@ private class UnusedInstanceCollector : JsVisitorWithContextImpl() {
private fun isLocalFunctionDeclaration(jsVar: JsVars.JsVar): Boolean {
val name = jsVar.name
val expr = jsVar.initExpression
val staticRef = name?.staticRef
return staticRef != null && staticRef == expr
// For the case like this: `b = a; c = b;`, where `a` is a function. In this case we should remove both declaration,
// although second one contains 'usage' of `b`.
// see `inlineEvaluationOrder/cases/lambdaWithClosure.kt`.
if (expr is JsNameRef && (expr.name?.let { tracker.isKnown(it) } ?: false)) return true
val staticRef = name?.staticRef
return staticRef != null && staticRef == expr && isFunctionReference(expr)
}
}
// For RHS of `var a = b;` checks whether *b* is a reference to a function or a closure instantiation, direct or indirect.
private fun isFunctionReference(expr: JsExpression): Boolean {
val qualifier = when (expr) {
// `var tmp = foo(closure)`, where `foo` is a closure constructor.
is JsInvocation -> expr.qualifier
// Either alias to another variable that holds function or a lambda without closure.
is JsNameRef -> expr
else -> null
}
return qualifier?.transitiveStaticRef is JsFunction
}
js/js.inliner/src/org/jetbrains/kotlin/js/inline/context/FunctionContext.kt
+11 -23
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2010-2015 JetBrains s.r.o.
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -17,11 +17,11 @@
package org.jetbrains.kotlin.js.inline.context
import com.google.dart.compiler.backend.js.ast.*
import com.google.dart.compiler.backend.js.ast.metadata.staticRef
import org.jetbrains.kotlin.js.inline.FunctionReader
import com.google.dart.compiler.backend.js.ast.metadata.descriptor
import org.jetbrains.kotlin.js.inline.util.*
import org.jetbrains.kotlin.js.inline.FunctionReader
import org.jetbrains.kotlin.js.inline.util.getSimpleName
import org.jetbrains.kotlin.js.inline.util.isCallInvocation
import org.jetbrains.kotlin.js.inline.util.transitiveStaticRef
abstract class FunctionContext(
private val function: JsFunction,
@@ -76,31 +76,19 @@ abstract class FunctionContext(
if (descriptor != null && descriptor in functionReader) return functionReader[descriptor]
/** remove ending `()` */
var callQualifier: JsNode = call.qualifier
/** remove ending `.call()` */
if (isCallInvocation(call)) {
callQualifier = (callQualifier as JsNameRef).qualifier!!
val callQualifier: JsExpression = if (isCallInvocation(call)) {
(call.qualifier as JsNameRef).qualifier!!
}
/** in case 4, 5 get ref (reduce 4, 5 to 2, 3 accordingly) */
@Suppress("USELESS_CAST") // NB do not remove 'as JsNode' below until KT-10752 is fixed
if (callQualifier is JsNameRef) {
val staticRef = callQualifier.name?.staticRef
callQualifier = when (staticRef) {
is JsNameRef -> staticRef as JsNode
is JsInvocation -> staticRef as JsNode
is JsFunction, null -> callQualifier
else -> throw AssertionError("Unexpected static reference type ${staticRef.javaClass}")
}
else {
call.qualifier
}
/** process cases 2, 3 */
val qualifier = callQualifier
val qualifier = callQualifier.transitiveStaticRef
return when (qualifier) {
is JsInvocation -> lookUpStaticFunction(getSimpleName(qualifier)!!)
is JsNameRef -> lookUpStaticFunction(qualifier.name)
is JsFunction -> qualifier
else -> null
}
}
js/js.inliner/src/org/jetbrains/kotlin/js/inline/context/NamingContext.kt
+3 -3
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2010-2015 JetBrains s.r.o.
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -27,7 +27,7 @@ import org.jetbrains.kotlin.js.inline.util.replaceNames
class NamingContext(
private val scope: JsScope,
private val statementContext: JsContext<JsStatement>
private val statementConsumer: (List<JsStatement>) -> Unit
) {
private val renamings = IdentityHashMap<JsName, JsExpression>()
private val declarations = ArrayList<JsVars>()
@@ -35,7 +35,7 @@ class NamingContext(
fun applyRenameTo(target: JsNode): JsNode {
if (!addedDeclarations) {
statementContext.addPrevious(declarations)
statementConsumer(declarations)
addedDeclarations = true
}
js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/invocationUtils.kt
+17 -1
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2010-2015 JetBrains s.r.o.
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -121,3 +121,19 @@ private fun getStaticRef(invocation: JsInvocation): JsNode? {
val qualifierName = (qualifier as? HasName)?.name
return qualifierName?.staticRef
}
/**
* If an expression A references to another expression B, which in turn references to C, or a corresponding expression
* at the end of a chain of references. They are usually JsNameRef expressions with JsFunction at the very end.
* This chain is produced when there are lots of aliases created from aliases, i.e. `var $tmp1 = foo; var $tmp2 = $tmp1;`.
* So for `$tmp2` we should get reference to `foo`.
*/
val JsExpression.transitiveStaticRef: JsExpression
get() {
var qualifier = this
while (qualifier is JsNameRef) {
val staticRef = qualifier.name?.staticRef as? JsExpression ?: break
qualifier = staticRef
}
return qualifier
}
js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/namingUtils.kt
+13 -10
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2010-2015 JetBrains s.r.o.
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
package org.jetbrains.kotlin.js.inline.util
import com.google.dart.compiler.backend.js.ast.*
import com.google.dart.compiler.backend.js.ast.metadata.staticRef
import org.jetbrains.kotlin.js.inline.context.NamingContext
import org.jetbrains.kotlin.js.inline.util.rewriters.LabelNameRefreshingVisitor
@@ -30,14 +31,16 @@ fun aliasArgumentsIfNeeded(
for ((arg, param) in arguments.zip(parameters)) {
val paramName = param.name
val replacement =
if (arg.needToAlias()) {
val freshName = context.getFreshName(paramName)
context.newVar(freshName, arg)
freshName.makeRef()
} else {
arg
}
val replacement: JsExpression = if (arg is JsFunction && isFunctionCreator(arg)) {
arg
}
else {
val freshName = context.getFreshName(paramName).apply { staticRef = arg }
context.newVar(freshName, arg)
val freshNameRef = freshName.makeRef()
freshNameRef
}
context.replaceName(paramName, replacement)
}
@@ -60,7 +63,7 @@ fun renameLocalNames(
function: JsFunction
) {
for (name in collectDefinedNames(function.body)) {
val freshName = context.getFreshName(name)
val freshName = context.getFreshName(name).apply { staticRef = name.staticRef }
context.replaceName(name, freshName.makeRef())
}
}
js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/rewriters/ReturnReplacingVisitor.kt
+9 -8
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2010-2015 JetBrains s.r.o.
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -20,7 +20,6 @@ import com.google.dart.compiler.backend.js.ast.*
import com.google.dart.compiler.backend.js.ast.metadata.functionDescriptor
import com.google.dart.compiler.backend.js.ast.metadata.returnTarget
import com.google.dart.compiler.backend.js.ast.metadata.synthetic
import org.jetbrains.kotlin.js.inline.util.canHaveSideEffect
import org.jetbrains.kotlin.js.translate.utils.JsAstUtils
class ReturnReplacingVisitor(
@@ -55,14 +54,16 @@ class ReturnReplacingVisitor(
}
private fun getReturnReplacement(returnExpression: JsExpression?): JsExpression? {
if (returnExpression != null) {
return if (returnExpression != null) {
if (resultRef != null) {
return JsAstUtils.assignment(resultRef, returnExpression).apply { synthetic = true }
JsAstUtils.assignment(resultRef, returnExpression).apply { synthetic = true }
}
else {
returnExpression
}
if (returnExpression.canHaveSideEffect()) return returnExpression
}
return null
else {
null
}
}
}
js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/sideEffectUtils.kt
+14 -28
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2010-2015 JetBrains s.r.o.
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -16,34 +16,20 @@
package org.jetbrains.kotlin.js.inline.util
import com.google.dart.compiler.backend.js.ast.JsLiteral.*
import com.google.dart.compiler.backend.js.ast.*
import com.google.dart.compiler.backend.js.ast.metadata.typeCheck
import com.google.dart.compiler.backend.js.ast.metadata.HasMetadata
import com.google.dart.compiler.backend.js.ast.metadata.sideEffects
import org.jetbrains.kotlin.js.translate.utils.jsAstUtils.any
fun JsExpression.canHaveSideEffect(): Boolean =
any { it is JsExpression && it.canHaveOwnSideEffect() }
fun JsExpression.canHaveSideEffect(localVars: Set<JsName>) =
any { it is JsExpression && it.canHaveOwnSideEffect(localVars) }
fun JsExpression.canHaveOwnSideEffect(): Boolean =
when (this) {
is JsValueLiteral,
is JsConditional,
is JsArrayAccess,
is JsArrayLiteral,
is JsNameRef -> false
is JsBinaryOperation -> operator.isAssignment
is JsInvocation -> !isFunctionCreatorInvocation(this)
else -> true
}
fun JsExpression.needToAlias(): Boolean =
any { it is JsExpression && it.shouldHaveOwnAlias() }
fun JsExpression.shouldHaveOwnAlias(): Boolean =
when (this) {
is JsConditional,
is JsBinaryOperation,
is JsArrayLiteral -> true
is JsInvocation -> if (typeCheck == null) canHaveSideEffect() else false
else -> canHaveOwnSideEffect()
}
fun JsExpression.canHaveOwnSideEffect(vars: Set<JsName>) = when (this) {
is JsConditional,
is JsLiteral -> false
is JsBinaryOperation -> operator.isAssignment
is JsNameRef -> !(qualifier == null && name in vars) && sideEffects
is JsInvocation -> !isFunctionCreatorInvocation(this) && sideEffects
is HasMetadata -> sideEffects
else -> true
}
js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/LiteralFunctionTranslator.kt
+2 -1
View File
@@ -19,6 +19,7 @@ package org.jetbrains.kotlin.js.translate.expression
import com.google.dart.compiler.backend.js.ast.*
import com.google.dart.compiler.backend.js.ast.metadata.functionDescriptor
import com.google.dart.compiler.backend.js.ast.metadata.isLocal
import com.google.dart.compiler.backend.js.ast.metadata.sideEffects
import com.google.dart.compiler.backend.js.ast.metadata.staticRef
import org.jetbrains.kotlin.descriptors.*
import org.jetbrains.kotlin.js.inline.util.getInnerFunction
@@ -62,7 +63,7 @@ class LiteralFunctionTranslator(context: TranslationContext) : AbstractTranslato
}
lambda.isLocal = true
return invokingContext.define(descriptor, lambda)
return invokingContext.define(descriptor, lambda).apply { sideEffects = false }
}
}
js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/functions/factories/KotlinFunctionIntrinsic.java
+4 -4
View File
@@ -1,5 +1,5 @@
/*
* Copyright 2010-2015 JetBrains s.r.o.
* Copyright 2010-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -18,7 +18,6 @@ package org.jetbrains.kotlin.js.translate.intrinsic.functions.factories;
import com.google.dart.compiler.backend.js.ast.JsExpression;
import com.google.dart.compiler.backend.js.ast.JsInvocation;
import com.google.dart.compiler.backend.js.ast.JsNameRef;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.kotlin.js.translate.context.Namer;
@@ -31,10 +30,10 @@ import java.util.List;
public class KotlinFunctionIntrinsic extends FunctionIntrinsic {
@NotNull
private final JsNameRef function;
private final String functionName;
public KotlinFunctionIntrinsic(@NotNull String functionName) {
function = JsAstUtils.fqnWithoutSideEffects(functionName, Namer.kotlinObject());
this.functionName = functionName;
}
@NotNull
@@ -44,6 +43,7 @@ public class KotlinFunctionIntrinsic extends FunctionIntrinsic {
@NotNull List<JsExpression> arguments,
@NotNull TranslationContext context
) {
JsExpression function = JsAstUtils.fqnWithoutSideEffects(functionName, Namer.kotlinObject());
return new JsInvocation(function, receiver == null ? arguments : TranslationUtils.generateInvocationArguments(receiver, arguments));
}
}