removed problematic expression "simplifications" (that introduced arbitrary r9 temp register usage)

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<IAstModification>? {
-            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

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<IAstModification> {
-        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)

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<IAstModification>()
-        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<IAstModification> {

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") {

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<List<String>
     return Pair(tmpvarName, decl)
 }
 
-fun getTempRegisterName(dt: InferredTypes.InferredType): List<String> {
-    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) {

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
 
 ...