From 93ce74eeb12a7895364f6511f55500482ef7e856 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sun, 7 Aug 2022 12:26:11 +0200 Subject: [PATCH] removed problematic expression "simplifications" (that introduced arbitrary r9 temp register usage) --- .../src/prog8/optimizer/StatementOptimizer.kt | 44 ++-------- .../astprocessing/AstOnetimeTransforms.kt | 2 +- .../astprocessing/BeforeAsmAstChanger.kt | 84 +------------------ compiler/test/TestTypecasts.kt | 2 +- compilerAst/src/prog8/ast/Extensions.kt | 14 ---- docs/source/todo.rst | 2 + 6 files changed, 12 insertions(+), 136 deletions(-) diff --git a/codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt b/codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt index cd750a5fe..974ff075e 100644 --- a/codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt +++ b/codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt @@ -1,6 +1,7 @@ package prog8.optimizer import prog8.ast.* +import prog8.ast.base.FatalAstException import prog8.ast.expressions.* import prog8.ast.statements.* import prog8.ast.walk.AstWalker @@ -81,26 +82,6 @@ class StatementOptimizer(private val program: Program, return listOf(IAstModification.Remove(functionCallStatement, parent as IStatementContainer)) } - if(compTarget.name!=VMTarget.NAME) { - // see if we can optimize any complex argument expressions to be just a simple variable - // TODO for now, only works for single-argument functions because we use just 1 temp var: R9 - if(functionCallStatement.target.nameInSource !in listOf(listOf("pop"), listOf("popw")) && functionCallStatement.args.size==1) { - val arg = functionCallStatement.args[0] - if(!arg.isSimple && arg !is IFunctionCall) { - val dt = arg.inferType(program) - if(dt.isInteger) { - val name = getTempRegisterName(dt) - val tempvar = IdentifierReference(name, functionCallStatement.position) - val assignTempvar = Assignment(AssignTarget(tempvar.copy(), null, null, functionCallStatement.position), arg, AssignmentOrigin.OPTIMIZER, functionCallStatement.position) - return listOf( - IAstModification.InsertBefore(functionCallStatement, assignTempvar, parent as IStatementContainer), - IAstModification.ReplaceNode(arg, tempvar, functionCallStatement) - ) - } - } - } - } - return noModifications } @@ -413,32 +394,21 @@ class StatementOptimizer(private val program: Program, if(compTarget.name==VMTarget.NAME) return noModifications - fun returnViaIntermediaryVar(value: Expression): Iterable? { - val subr = returnStmt.definingSubroutine!! - val returnDt = subr.returntypes.single() - if (returnDt in IntegerDatatypes) { + val returnvalue = returnStmt.value + if (returnvalue!=null) { + val dt = returnvalue.inferType(program).getOrElse { throw FatalAstException("invalid dt") } + if (returnvalue is BinaryExpression || (returnvalue is TypecastExpression && !returnvalue.expression.isSimple)) { // first assign to intermediary variable, then return that - val (returnVarName, _) = program.getTempVar(returnDt) + val (returnVarName, _) = program.getTempVar(dt) val returnValueIntermediary = IdentifierReference(returnVarName, returnStmt.position) val tgt = AssignTarget(returnValueIntermediary, null, null, returnStmt.position) - val assign = Assignment(tgt, value, AssignmentOrigin.OPTIMIZER, returnStmt.position) + val assign = Assignment(tgt, returnvalue, AssignmentOrigin.OPTIMIZER, returnStmt.position) val returnReplacement = Return(returnValueIntermediary.copy(), returnStmt.position) return listOf( IAstModification.InsertBefore(returnStmt, assign, parent as IStatementContainer), IAstModification.ReplaceNode(returnStmt, returnReplacement, parent) ) } - return null - } - - // TODO decision when to use intermediary variable to calculate returnvalue seems a bit arbitrary... - val returnvalue = returnStmt.value - if (returnvalue!=null) { - if (returnvalue is BinaryExpression || (returnvalue is TypecastExpression && !returnvalue.expression.isSimple)) { - val mod = returnViaIntermediaryVar(returnvalue) - if(mod!=null) - return mod - } } return noModifications diff --git a/compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt b/compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt index 11ef786f4..c95cc3fec 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt @@ -29,7 +29,7 @@ internal class AstOnetimeTransforms(private val program: Program, private val op } private fun replacePointerVarIndexWithMemreadOrMemwrite(arrayIndexedExpression: ArrayIndexedExpression, parent: Node): Iterable { - if(options.compTarget.name== VMTarget.NAME) + if(options.compTarget.name==VMTarget.NAME) return noModifications // vm codegen deals correctly with all cases val arrayVar = arrayIndexedExpression.arrayvar.targetVarDecl(program) diff --git a/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt b/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt index f61b07504..e7b195722 100644 --- a/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt +++ b/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt @@ -166,7 +166,6 @@ internal class BeforeAsmAstChanger(val program: Program, // (non-asm routines get a Return statement as needed, above) val instruction = if(options.compTarget.name==VMTarget.NAME) " return\n" else " rts\n" mods += IAstModification.InsertLast(InlineAssembly(instruction, Position.DUMMY), subroutine) - println("adding returnstmt3 ${subroutine.hasRtsInAsm(options.compTarget)}") // TODO } } @@ -217,88 +216,7 @@ internal class BeforeAsmAstChanger(val program: Program, (binExpr.right as? NumericLiteral)?.number!=0.0) throw InternalCompilerException("0==X should have been swapped to if X==0") - // simplify the conditional expression, introduce simple assignments if required. - // NOTE: sometimes this increases code size because additional stores/loads are generated for the - // intermediate variables. We assume these are optimized away from the resulting assembly code later. - val simplify = simplifyConditionalExpression(binExpr) - val modifications = mutableListOf() - if(simplify.rightVarAssignment!=null) { - modifications += IAstModification.ReplaceNode(binExpr.right, simplify.rightOperandReplacement!!, binExpr) - modifications += IAstModification.InsertBefore( - ifElse, - simplify.rightVarAssignment, - parent as IStatementContainer - ) - } - if(simplify.leftVarAssignment!=null) { - modifications += IAstModification.ReplaceNode(binExpr.left, simplify.leftOperandReplacement!!, binExpr) - modifications += IAstModification.InsertBefore( - ifElse, - simplify.leftVarAssignment, - parent as IStatementContainer - ) - } - - return modifications - } - - private class CondExprSimplificationResult( - val leftVarAssignment: Assignment?, - val leftOperandReplacement: Expression?, - val rightVarAssignment: Assignment?, - val rightOperandReplacement: Expression? - ) - - private fun simplifyConditionalExpression(expr: BinaryExpression): CondExprSimplificationResult { - - // TODO: somehow figure out if the expr will result in stack-evaluation STILL after being split off, - // in that case: do *not* split it off but just keep it as it is (otherwise code size increases) - // NOTE: do NOT move this to an earler ast transform phase (such as StatementReorderer or StatementOptimizer) - it WILL result in larger code. - - if(options.compTarget.name==VMTarget.NAME) // don't apply this optimization for Vm target - return CondExprSimplificationResult(null, null, null, null) - - var leftAssignment: Assignment? = null - var leftOperandReplacement: Expression? = null - var rightAssignment: Assignment? = null - var rightOperandReplacement: Expression? = null - - val separateLeftExpr = !expr.left.isSimple - && expr.left !is IFunctionCall - && expr.left !is ContainmentCheck - val separateRightExpr = !expr.right.isSimple - && expr.right !is IFunctionCall - && expr.right !is ContainmentCheck - val leftDt = expr.left.inferType(program) - val rightDt = expr.right.inferType(program) - - if(!leftDt.isInteger || !rightDt.isInteger) { - // we can't reasonably simplify non-integer expressions - return CondExprSimplificationResult(null, null, null, null) - } - - if(separateLeftExpr) { - val name = getTempRegisterName(leftDt) - leftOperandReplacement = IdentifierReference(name, expr.position) - leftAssignment = Assignment( - AssignTarget(IdentifierReference(name, expr.position), null, null, expr.position), - expr.left, - AssignmentOrigin.BEFOREASMGEN, expr.position - ) - } - if(separateRightExpr) { - val (tempVarName, _) = program.getTempVar(rightDt.getOrElse { throw FatalAstException("invalid dt") }, true) - rightOperandReplacement = IdentifierReference(tempVarName, expr.position) - rightAssignment = Assignment( - AssignTarget(IdentifierReference(tempVarName, expr.position), null, null, expr.position), - expr.right, - AssignmentOrigin.BEFOREASMGEN, expr.position - ) - } - return CondExprSimplificationResult( - leftAssignment, leftOperandReplacement, - rightAssignment, rightOperandReplacement - ) + return noModifications } override fun after(arrayIndexedExpression: ArrayIndexedExpression, parent: Node): Iterable { diff --git a/compiler/test/TestTypecasts.kt b/compiler/test/TestTypecasts.kt index 8b03bf8a5..e72886d29 100644 --- a/compiler/test/TestTypecasts.kt +++ b/compiler/test/TestTypecasts.kt @@ -827,7 +827,7 @@ main { """ val result = compileText(C64Target(), false, text, writeAssembly = true)!! val statements = result.program.entrypoint.statements - statements.size shouldBe 27 + statements.size shouldBeGreaterThan 10 } test("cast to unsigned in conditional") { diff --git a/compilerAst/src/prog8/ast/Extensions.kt b/compilerAst/src/prog8/ast/Extensions.kt index dcf0aab07..8b18586ad 100644 --- a/compilerAst/src/prog8/ast/Extensions.kt +++ b/compilerAst/src/prog8/ast/Extensions.kt @@ -3,7 +3,6 @@ package prog8.ast import prog8.ast.base.FatalAstException import prog8.ast.expressions.BinaryExpression import prog8.ast.expressions.Expression -import prog8.ast.expressions.InferredTypes import prog8.ast.statements.VarDecl import prog8.ast.statements.VarDeclOrigin import prog8.ast.statements.VarDeclType @@ -48,19 +47,6 @@ fun Program.getTempVar(dt: DataType, altNames: Boolean=false): Pair return Pair(tmpvarName, decl) } -fun getTempRegisterName(dt: InferredTypes.InferredType): List { - return when { - // TODO assume (hope) cx16.r9 isn't used for anything else during the use of this temporary variable... - dt istype DataType.UBYTE -> listOf("cx16", "r9L") - dt istype DataType.BOOL -> listOf("cx16", "r9L") - dt istype DataType.BYTE -> listOf("cx16", "r9sL") - dt istype DataType.UWORD -> listOf("cx16", "r9") - dt istype DataType.WORD -> listOf("cx16", "r9s") - dt.isPassByReference -> listOf("cx16", "r9") - else -> throw FatalAstException("invalid dt $dt") - } -} - fun maySwapOperandOrder(binexpr: BinaryExpression): Boolean { fun ok(expr: Expression): Boolean { return when(expr) { diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 2a805cb9b..b17ea66f1 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,6 +3,8 @@ TODO For next release ^^^^^^^^^^^^^^^^ +- fix the obvious TODOs in CodeGen (vm) +- vm: AssignmentGen - fix in-place array assign TODO ...