From 85f7c174cd12bf3a9d3c375392874826278f90e8 Mon Sep 17 00:00:00 2001 From: Michael Bogdanov Date: Sun, 27 Sep 2015 11:59:12 +0300 Subject: [PATCH] First implementation of proper agrument order --- .../kotlin/codegen/ArgumentGenerator.kt | 61 ++++++++++++++----- .../codegen/CallBasedArgumentGenerator.java | 20 +++--- .../kotlin/codegen/ExpressionCodegen.java | 4 +- .../codegen/ImplementationBodyCodegen.java | 3 + .../jetbrains/kotlin/codegen/StackValue.java | 2 +- .../kotlin/codegen/inline/InlineCodegen.java | 33 +++++----- 6 files changed, 79 insertions(+), 44 deletions(-) diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ArgumentGenerator.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/ArgumentGenerator.kt index e93fe73c8d4..66e6a90b156 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ArgumentGenerator.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ArgumentGenerator.kt @@ -16,14 +16,13 @@ package org.jetbrains.kotlin.codegen -import org.jetbrains.kotlin.resolve.calls.model.DefaultValueArgument -import org.jetbrains.kotlin.resolve.calls.model.ExpressionValueArgument -import org.jetbrains.kotlin.resolve.calls.model.ResolvedValueArgument -import org.jetbrains.kotlin.resolve.calls.model.VarargValueArgument +import org.jetbrains.kotlin.resolve.calls.model.* import org.jetbrains.kotlin.utils.mapToIndex import org.jetbrains.org.objectweb.asm.Type import java.util.* +private class ArgumentAndIndex(val arg: ResolvedValueArgument, val declIndex: Int, var type: Type? = null, var reoder: Boolean = false, var tempValue: StackValue? = null) + abstract class ArgumentGenerator { /** * @return a `List` of bit masks of default arguments that should be passed as last arguments to $default method, if there were @@ -34,7 +33,7 @@ abstract class ArgumentGenerator { * * * @param actualArgs */ - open fun generate(valueArgumentsByIndex: List, actualArgs: List): DefaultCallMask { + open fun generate(valueArgumentsByIndex: List, actualArgs: List, codegen: ExpressionCodegen): DefaultCallMask { //HACK: see tempVariable in ExpressionCodegen val actualArguments = if (actualArgs.isNotEmpty()) actualArgs else valueArgumentsByIndex @@ -44,29 +43,61 @@ abstract class ArgumentGenerator { val arg2Index = valueArgumentsByIndex.mapToIndex() - val masks = DefaultCallMask(valueArgumentsByIndex.size()) - for ((index, argument) in valueArgumentsByIndex.withIndex()) { - //var i = arg2Index[argument]!! - var i = index + val actualArgsWithDeclIndex: ArrayList = ArrayList(actualArguments.filter { it !is DefaultValueArgument }.map { + ArgumentAndIndex(it, arg2Index[it]!!) + }) - var type = when (argument) { + valueArgumentsByIndex.withIndex().forEach { + if (it.value is DefaultValueArgument) { + actualArgsWithDeclIndex.add(it.index, ArgumentAndIndex(it.value, it.index)) + } + } + + val masks = DefaultCallMask(valueArgumentsByIndex.size()) + var orderChanged = false + + for ((actualIndex, argumentWithDeclIndex) in actualArgsWithDeclIndex.withIndex()) { + val argument = argumentWithDeclIndex.arg + val declIndex = argumentWithDeclIndex.declIndex + + argumentWithDeclIndex.type = when (argument) { is ExpressionValueArgument -> { - generateExpression(i, argument) + generateExpression(declIndex, argument) } is DefaultValueArgument -> { - masks.mark(i) - generateDefault(i, argument) + masks.mark(declIndex) + generateDefault(declIndex, argument) } is VarargValueArgument -> { - generateVararg(i, argument) + generateVararg(declIndex, argument) } else -> { - generateOther(i, argument) + generateOther(declIndex, argument) } } + if (actualIndex != declIndex || orderChanged) { + orderChanged = true + argumentWithDeclIndex.reoder = true + } } + val mark = codegen.myFrameMap.mark() + actualArgsWithDeclIndex.reversed().forEach { + if (it.reoder) { + val type = it.type!! + it.tempValue = StackValue.local(codegen.frameMap.enterTemp(type), type) + it.tempValue?.store(StackValue.onStack(type), codegen.v) + } + } + + actualArgsWithDeclIndex.sortedBy { it.declIndex }.forEach { + it.tempValue?.let { + it.put(it.type, codegen.v) + } + } + + mark.dropTo() return masks } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/CallBasedArgumentGenerator.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/CallBasedArgumentGenerator.java index 2e6597bb8c0..c399ef60af4 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/CallBasedArgumentGenerator.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/CallBasedArgumentGenerator.java @@ -20,10 +20,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.kotlin.descriptors.ValueParameterDescriptor; import org.jetbrains.kotlin.psi.JetExpression; import org.jetbrains.kotlin.psi.ValueArgument; -import org.jetbrains.kotlin.resolve.calls.model.DefaultValueArgument; -import org.jetbrains.kotlin.resolve.calls.model.ExpressionValueArgument; -import org.jetbrains.kotlin.resolve.calls.model.ResolvedValueArgument; -import org.jetbrains.kotlin.resolve.calls.model.VarargValueArgument; +import org.jetbrains.kotlin.resolve.calls.model.*; import org.jetbrains.org.objectweb.asm.Type; import java.util.List; @@ -51,13 +48,18 @@ public class CallBasedArgumentGenerator extends ArgumentGenerator { "Value parameters and their types mismatch in sizes: " + valueParameters.size() + " != " + valueParameterTypes.size(); } + @NotNull @Override - public DefaultCallMask generate(@NotNull List valueArgumentsByIndex, @NotNull List valueArgs) { - boolean shouldMarkLineNumbers = codegen.isShouldMarkLineNumbers(); - codegen.setShouldMarkLineNumbers(false); - DefaultCallMask masks = super.generate(valueArgumentsByIndex, valueArgs); - codegen.setShouldMarkLineNumbers(shouldMarkLineNumbers); + public DefaultCallMask generate( + @NotNull List valueArgumentsByIndex, + @NotNull List valueArgs, + @NotNull ExpressionCodegen codegen + ) { + boolean shouldMarkLineNumbers = this.codegen.isShouldMarkLineNumbers(); + this.codegen.setShouldMarkLineNumbers(false); + DefaultCallMask masks = super.generate(valueArgumentsByIndex, valueArgs, codegen); + this.codegen.setShouldMarkLineNumbers(shouldMarkLineNumbers); return masks; } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java index b690b8f9c1d..317bc03f681 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java @@ -1486,7 +1486,7 @@ public class ExpressionCodegen extends JetVisitor implem ArgumentGenerator argumentGenerator = new CallBasedArgumentGenerator(ExpressionCodegen.this, defaultCallGenerator, valueParameters, mappedTypes); - argumentGenerator.generate(valueArguments, valueArguments); + argumentGenerator.generate(valueArguments, valueArguments, ExpressionCodegen.this); } Collection constructors = classDescriptor.getConstructors(); @@ -2455,7 +2455,7 @@ public class ExpressionCodegen extends JetVisitor implem assert valueArguments != null : "Failed to arrange value arguments by index: " + resolvedCall.getResultingDescriptor(); List masks = - argumentGenerator.generate(valueArguments, new ArrayList(resolvedCall.getValueArguments().values())) + argumentGenerator.generate(valueArguments, new ArrayList(resolvedCall.getValueArguments().values()), this) .toInts(); if (tailRecursionCodegen.isTailRecursion(resolvedCall)) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java index 6ba72975c9f..0cbef0df19d 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java @@ -1643,11 +1643,13 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { this.offset = firstValueParamOffset; } + @NotNull @Override public Type generateExpression(int i, @NotNull ExpressionValueArgument argument) { return generateSuperCallArgument(i); } + @NotNull @Override public Type generateDefault(int i, @NotNull DefaultValueArgument argument) { Type type = parameters.get(i).getAsmType(); @@ -1655,6 +1657,7 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { return type; } + @NotNull @Override public Type generateVararg(int i, @NotNull VarargValueArgument argument) { return generateSuperCallArgument(i); diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java index d2b69cb23b4..548a1758c04 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java @@ -769,7 +769,7 @@ public abstract class StackValue { ResolvedCall call = isGetter ? resolvedGetCall : resolvedSetCall; StackValue newReceiver = StackValue.receiver(call, receiver, codegen, callable); newReceiver.put(newReceiver.type, v); - argumentGenerator.generate(valueArguments, valueArguments); + argumentGenerator.generate(valueArguments, valueArguments, codegen); } @Override diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineCodegen.java index c4a043c4898..05bf5bf7170 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/inline/InlineCodegen.java @@ -40,7 +40,6 @@ import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils; import org.jetbrains.kotlin.resolve.DescriptorUtils; import org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilKt; import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall; -import org.jetbrains.kotlin.resolve.inline.InlineStrategy; import org.jetbrains.kotlin.resolve.inline.InlineUtil; import org.jetbrains.kotlin.resolve.jvm.AsmTypes; import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmMethodParameterKind; @@ -84,8 +83,8 @@ public class InlineCodegen extends CallGenerator { private final int initialFrameSize; private final boolean isSameModule; - protected final ParametersBuilder invocationParamBuilder = ParametersBuilder.newBuilder(); - protected final Map expressionMap = new HashMap(); + private final ParametersBuilder invocationParamBuilder = ParametersBuilder.newBuilder(); + private final Map expressionMap = new HashMap(); private final ReifiedTypeInliner reifiedTypeInliner; @@ -116,7 +115,6 @@ public class InlineCodegen extends CallGenerator { jvmSignature = typeMapper.mapSignature(functionDescriptor, context.getContextKind()); // TODO: implement AS_FUNCTION inline strategy - InlineStrategy inlineStrategy = InlineUtil.getInlineStrategy(functionDescriptor); this.asFunctionInline = false; isSameModule = JvmCodegenUtil.isCallInsideSameModuleAsDeclared(functionDescriptor, codegen.getContext(), state.getOutDirectory()); @@ -413,15 +411,17 @@ public class InlineCodegen extends CallGenerator { @Override public void afterParameterPut(@NotNull Type type, @Nullable StackValue stackValue, @Nullable ValueParameterDescriptor valueParameterDescriptor) { - putCapturedInLocal(type, stackValue, valueParameterDescriptor, -1); + putArgumentOrCapturedToLocalVal(type, stackValue, -1); } - private void putCapturedInLocal( - @NotNull Type type, @Nullable StackValue stackValue, @Nullable ValueParameterDescriptor valueParameterDescriptor, int capturedParamIndex + private void putArgumentOrCapturedToLocalVal( + @NotNull Type type, + @Nullable StackValue stackValue, + int capturedParamIndex ) { if (!asFunctionInline && Type.VOID_TYPE != type) { //TODO remap only inlinable closure => otherwise we could get a lot of problem - boolean couldBeRemapped = !shouldPutValue(type, stackValue, valueParameterDescriptor); + boolean couldBeRemapped = !shouldPutValue(type, stackValue); StackValue remappedIndex = couldBeRemapped ? stackValue : null; ParameterInfo info; @@ -434,15 +434,14 @@ public class InlineCodegen extends CallGenerator { info = invocationParamBuilder.addNextParameter(type, false, remappedIndex); } - putParameterOnStack(info); + recordParameterValueInLocalVal(info); } } /*descriptor is null for captured vars*/ - public boolean shouldPutValue( + public static boolean shouldPutValue( @NotNull Type type, - @Nullable StackValue stackValue, - @Nullable ValueParameterDescriptor descriptor + @Nullable StackValue stackValue ) { if (stackValue == null) { @@ -478,7 +477,7 @@ public class InlineCodegen extends CallGenerator { return true; } - private void putParameterOnStack(ParameterInfo... infos) { + private void recordParameterValueInLocalVal(ParameterInfo... infos) { int[] index = new int[infos.length]; for (int i = 0; i < infos.length; i++) { ParameterInfo info = infos[i]; @@ -515,7 +514,7 @@ public class InlineCodegen extends CallGenerator { } List infos = invocationParamBuilder.listNotCaptured(); - putParameterOnStack(infos.toArray(new ParameterInfo[infos.size()])); + recordParameterValueInLocalVal(infos.toArray(new ParameterInfo[infos.size()])); } public void leaveTemps() { @@ -633,7 +632,7 @@ public class InlineCodegen extends CallGenerator { @Override public void putValueIfNeeded(@Nullable ValueParameterDescriptor valueParameterDescriptor, @NotNull Type parameterType, @NotNull StackValue value) { - if (shouldPutValue(parameterType, value, valueParameterDescriptor)) { + if (shouldPutValue(parameterType, value)) { value.put(parameterType, codegen.v); } afterParameterPut(parameterType, value, valueParameterDescriptor); @@ -643,10 +642,10 @@ public class InlineCodegen extends CallGenerator { public void putCapturedValueOnStack( @NotNull StackValue stackValue, @NotNull Type valueType, int paramIndex ) { - if (shouldPutValue(stackValue.type, stackValue, null)) { + if (shouldPutValue(stackValue.type, stackValue)) { stackValue.put(stackValue.type, codegen.v); } - putCapturedInLocal(stackValue.type, stackValue, null, paramIndex); + putArgumentOrCapturedToLocalVal(stackValue.type, stackValue, paramIndex); }