tweak BinaryExpression splitting

codeGenCpu6502/src/prog8/codegen/cpu6502/ExpressionsAsmGen.kt

@@ -11,7 +11,7 @@ internal class ExpressionsAsmGen(private val program: PtProgram,
     @Deprecated("avoid calling this as it generates slow evalstack based code")
     internal fun translateExpression(expression: PtExpression) {
         if (this.asmgen.options.slowCodegenWarnings) {
-            asmgen.errors.warn("slow stack evaluation used for expression $expression", expression.position)
+            asmgen.errors.warn("slow stack evaluation used for expression", expression.position)
         }
         translateExpressionInternal(expression)
     }

codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt

@@ -2,7 +2,9 @@ package prog8.codegen.cpu6502.assignment
 
 import prog8.code.ast.*
 import prog8.code.core.*
-import prog8.codegen.cpu6502.*
+import prog8.codegen.cpu6502.AsmGen6502Internal
+import prog8.codegen.cpu6502.VariableAllocator
+import prog8.codegen.cpu6502.returnsWhatWhere
 
 
 internal class AssignmentAsmGen(private val program: PtProgram,
@@ -793,9 +795,7 @@ internal class AssignmentAsmGen(private val program: PtProgram,
     }
 
     private fun fallbackToStackEval(assign: AsmAssignment) {
-        // this routine is called for assigning a binaryexpression value:
+        // this routine is called for assigning a binaryexpression value that has no optimized code path.
-        // - if it's a boolean comparison expression and the workaround isn't possible (no origTarget ast node)
-        // - for all other binary expressions.
         asmgen.translateExpression(assign.source.expression!!)
         if (assign.target.datatype in WordDatatypes && assign.source.datatype in ByteDatatypes)
             asmgen.signExtendStackLsb(assign.source.datatype)

codeOptimizers/src/prog8/optimizer/BinExprSplitter.kt

@@ -4,9 +4,6 @@ import prog8.ast.IStatementContainer
 import prog8.ast.Node
 import prog8.ast.Program
 import prog8.ast.expressions.BinaryExpression
-import prog8.ast.expressions.IdentifierReference
-import prog8.ast.expressions.TypecastExpression
-import prog8.ast.getTempVar
 import prog8.ast.statements.AssignTarget
 import prog8.ast.statements.Assignment
 import prog8.ast.statements.AssignmentOrigin
@@ -31,23 +28,6 @@ class BinExprSplitter(private val program: Program, private val options: Compila
         val binExpr = assignment.value as? BinaryExpression
         if (binExpr != null) {
 
-/*
-
-Reduce the complexity of a (binary) expression that has to be evaluated on the eval stack,
-by attempting to splitting it up into individual simple steps.
-We only consider a binary expression *one* level deep (so the operands must not be a combined expression)
-
-
-X =      BinExpr                                    X   =   LeftExpr
-        <operator>                                     followed by
-          /   \             IF 'X' not used         X   =   BinExpr
-         /     \             IN expression ==>             <operator>
-        /       \                                           /   \
-    LeftExpr.  RightExpr.                                  /     \
-                                                          X     RightExpr.
-
-
- */
             if(binExpr.operator in AugmentAssignmentOperators && isSimpleTarget(assignment.target)) {
                 if(assignment.target isSameAs binExpr.right)
                     return noModifications
@@ -60,20 +40,6 @@ X =      BinExpr                                    X   =   LeftExpr
                     val rightBx = binExpr.right as? BinaryExpression
                     if(rightBx!=null && (!rightBx.left.isSimple || !rightBx.right.isSimple))
                         return noModifications
-
-                      // TODO below attempts to remove stack-based evaluated expressions, but often the resulting code is BIGGER, and SLOWER.
-//                    val dt = assignment.target.inferType(program)
-//                    if(!dt.isInteger)
-//                        return noModifications
-//                    val tempVar = IdentifierReference(getTempVarName(dt), binExpr.right.position)
-//                    val assignTempVar = Assignment(
-//                        AssignTarget(tempVar, null, null, binExpr.right.position),
-//                        binExpr.right, binExpr.right.position
-//                    )
-//                    return listOf(
-//                        IAstModification.InsertBefore(assignment, assignTempVar, assignment.parent as IStatementContainer),
-//                        IAstModification.ReplaceNode(binExpr.right, tempVar.copy(), binExpr)
-//                    )
                 }
 
                 if(binExpr.right.isSimple) {
@@ -87,30 +53,10 @@ X =      BinExpr                                    X   =   LeftExpr
                 }
             }
 
-            // TODO further unraveling of binary expression trees into flat statements.
+            // Further unraveling of binary expressions is really complicated here and
-            // however this should probably be done in a more generic way to also work on
+            // often results in much bigger code, thereby defeating the purpose a bit.
-            // the expressiontrees that are not used in an assignment statement...
+            // All in all this should probably be fixed in a better code generation backend
-        }
+            // that doesn't require this at all.
-
-        val typecast = assignment.value as? TypecastExpression
-        if(typecast!=null) {
-            val origExpr = typecast.expression as? BinaryExpression
-            if(origExpr!=null && options.compTarget.name!=VMTarget.NAME) {
-                // it's a typecast of a binary expression.
-                // we can see if we can unwrap the binary expression by working on a new temporary variable
-                // (that has the type of the expression), and then finally doing the typecast.
-                // Once it's outside the typecast, the regular splitting can commence.
-                val tempvarDt = origExpr.inferType(program).getOr(DataType.UNDEFINED)
-                val (tempVarName, _) = program.getTempVar(tempvarDt)
-                val assignTempVar = Assignment(
-                    AssignTarget(IdentifierReference(tempVarName, typecast.position), null, null, typecast.position),
-                    typecast.expression, AssignmentOrigin.OPTIMIZER, typecast.position
-                )
-                return listOf(
-                    IAstModification.InsertBefore(assignment, assignTempVar, parent as IStatementContainer),
-                    IAstModification.ReplaceNode(typecast.expression, IdentifierReference(tempVarName, typecast.position), typecast)
-                )
-            }
         }
 
         return noModifications

compiler/test/TestOptimization.kt

@@ -276,39 +276,6 @@ class TestOptimization: FunSpec({
         value3.operator shouldBe "&"
     }
 
-    test("intermediate assignment steps generated for typecasted expression") {
-        val src = """
-            main {
-                sub start() {
-                    ubyte r
-                    ubyte @shared bb = (sgn(r)*2 + 100) as ubyte
-                }
-            }
-        """
-        val result = compileText(C64Target(), true, src, writeAssembly = true)!!
-        /* turned into:
-        ubyte r
-        r = 0
-        ubyte bb
-        prog8_lib.retval_interm_b = sgn(r)
-        prog8_lib.retval_interm_b <<= 1
-        prog8_lib.retval_interm_b += 100
-        bb = prog8_lib.retval_interm_b
-        return
-         */
-        val st = result.compilerAst.entrypoint.statements
-        st.size shouldBe 8
-        st.last() shouldBe instanceOf<Return>()
-        var assign = st[3] as Assignment
-        assign.target.identifier!!.nameInSource shouldBe listOf("prog8_lib","tempvar_b")
-        assign = st[4] as Assignment
-        assign.target.identifier!!.nameInSource shouldBe listOf("prog8_lib","tempvar_b")
-        assign = st[5] as Assignment
-        assign.target.identifier!!.nameInSource shouldBe listOf("prog8_lib","tempvar_b")
-        assign = st[6] as Assignment
-        assign.target.identifier!!.nameInSource shouldBe listOf("bb")
-    }
-
     test("asmgen correctly deals with float typecasting in augmented assignment") {
         val src="""
             %import floats

docs/source/todo.rst

@@ -8,10 +8,11 @@ For next minor release
 
 For 9.0 major changes
 ^^^^^^^^^^^^^^^^^^^^^
+- get rid of the disknumber parameter everywhere in diskio, make it a configurable variable that defaults to 8.
+  the large majority of users will only deal with a single disk drive so why not make it easier for them.
 - duplicate diskio for cx16 (get rid of cx16diskio, just copy diskio and tweak everything) + documentation
-- get rid of the disknumber parameter everywhere in diskio, make it a configurable variable that defaults to 8
 - get f_seek_w working like in the BASIC program  - this needs the changes to diskio.f_open to use suffixes ,p,m
-- Some support for (64tass) SEGMENTS ?
+- Some more support for (64tass) SEGMENTS ?
     - Add a mechanism to allocate variables into golden ram (or segments really) (see GoldenRam class)
     - maybe treat block "golden" in a special way: can only contain vars, every var will be allocated in the Golden ram area?
     - the variables can NOT have initialization values, they will all be set to zero on startup (simple memset)
@@ -74,9 +75,9 @@ Expressions:
 Optimizations:
 
 - VariableAllocator: can we think of a smarter strategy for allocating variables into zeropage, rather than first-come-first-served?
-- when a loopvariable of a forloop isn't referenced in the body, and the iterations are known, replace the loop by a repeatloop
+  for instance, vars used inside loops first, then loopvars, then the rest
+- when a loopvariable of a forloop isn't referenced in the body, and the number of iterations is known, replace the loop by a repeatloop
   but we have no efficient way right now to see if the body references a variable.
-- optimize function argument expressions better (use temporary variables to replace non-simple expressions?)
 - various optimizers skip stuff if compTarget.name==VMTarget.NAME.  Once 6502-codegen is done from IR code,
   those checks should probably be removed, or be made permanent