preparing optimization plan for if statements

compiler/src/prog8/compiler/target/cpu6502/codegen/ExpressionsAsmGen.kt

@@ -65,7 +65,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge
                 if(rightConstVal?.number?.toDouble() == 0.0) {
                     if(dt in ByteDatatypes) {
                         asmgen.assignExpressionToRegister(left, RegisterOrPair.A)
-                        if(left is FunctionCall)
+                        if(left is FunctionCall && !left.isSimple)
                             asmgen.out("  cmp  #0")
                         asmgen.out("  bne  $jumpIfFalseLabel")
                         return
@@ -94,7 +94,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge
                 if(rightConstVal?.number?.toDouble() == 0.0) {
                     if(dt in ByteDatatypes) {
                         asmgen.assignExpressionToRegister(left, RegisterOrPair.A)
-                        if(left is FunctionCall)
+                        if(left is FunctionCall && !left.isSimple)
                             asmgen.out("  cmp  #0")
                         asmgen.out("  beq  $jumpIfFalseLabel")
                         return

compiler/src/prog8/optimizer/BinExprSplitter.kt

@@ -57,7 +57,7 @@ X =      BinExpr                                    X   =   LeftExpr
                 if(assignment.target isSameAs binExpr.left || assignment.target isSameAs binExpr.right)
                     return noModifications
 
-                if(isSimpleExpression(binExpr.right) && !assignment.isAugmentable) {
+                if(binExpr.right.isSimple && !assignment.isAugmentable) {
                     val firstAssign = Assignment(assignment.target.copy(), binExpr.left, binExpr.left.position)
                     val targetExpr = assignment.target.toExpression()
                     val augExpr = BinaryExpression(targetExpr, binExpr.operator, binExpr.right, binExpr.right.position)
@@ -76,9 +76,6 @@ X =      BinExpr                                    X   =   LeftExpr
         return noModifications
     }
 
-    private fun isSimpleExpression(expr: Expression) =
-            expr is IdentifierReference || expr is NumericLiteralValue || expr is AddressOf || expr is DirectMemoryRead || expr is StringLiteralValue || expr is ArrayLiteralValue || expr is RangeExpr
-
     private fun isSimpleTarget(target: AssignTarget, program: Program) =
             if (target.identifier!=null || target.memoryAddress!=null)
                 compTarget.isInRegularRAM(target, program)

compilerAst/src/prog8/ast/expressions/AstExpressions.kt

@@ -22,6 +22,7 @@ sealed class Expression: Node {
     abstract fun accept(visitor: AstWalker, parent: Node)
     abstract fun referencesIdentifier(vararg scopedName: String): Boolean
     abstract fun inferType(program: Program): InferredTypes.InferredType
+    abstract val isSimple: Boolean
 
     infix fun isSameAs(assigntarget: AssignTarget) = assigntarget.isSameAs(this)
 
@@ -105,6 +106,8 @@ class PrefixExpression(val operator: String, var expression: Expression, overrid
         }
     }
 
+    override val isSimple = false
+
     override fun toString(): String {
         return "Prefix($operator $expression)"
     }
@@ -133,6 +136,8 @@ class BinaryExpression(var left: Expression, var operator: String, var right: Ex
         return "[$left $operator $right]"
     }
 
+    override val isSimple = false
+
     // binary expression should actually have been optimized away into a single value, before const value was requested...
     override fun constValue(program: Program): NumericLiteralValue? = null
 
@@ -242,6 +247,8 @@ class ArrayIndexedExpression(var arrayvar: IdentifierReference,
         indexer.linkParents(this)
     }
 
+    override val isSimple = false
+
     override fun replaceChildNode(node: Node, replacement: Node) {
         when {
             node===arrayvar -> arrayvar = replacement as IdentifierReference
@@ -283,6 +290,8 @@ class TypecastExpression(var expression: Expression, var type: DataType, val imp
         expression.linkParents(this)
     }
 
+    override val isSimple = false
+
     override fun replaceChildNode(node: Node, replacement: Node) {
         require(replacement is Expression && node===expression)
         expression = replacement
@@ -316,6 +325,8 @@ data class AddressOf(var identifier: IdentifierReference, override val position:
         identifier.parent=this
     }
 
+    override val isSimple = true
+
     override fun replaceChildNode(node: Node, replacement: Node) {
         require(replacement is IdentifierReference && node===identifier)
         identifier = replacement
@@ -337,6 +348,8 @@ class DirectMemoryRead(var addressExpression: Expression, override val position:
         this.addressExpression.linkParents(this)
     }
 
+    override val isSimple = true
+
     override fun replaceChildNode(node: Node, replacement: Node) {
         require(replacement is Expression && node===addressExpression)
         addressExpression = replacement
@@ -362,6 +375,8 @@ class NumericLiteralValue(val type: DataType,    // only numerical types allowed
                           override val position: Position) : Expression() {
     override lateinit var parent: Node
 
+    override val isSimple = true
+
     companion object {
         fun fromBoolean(bool: Boolean, position: Position) =
                 NumericLiteralValue(DataType.UBYTE, if (bool) 1 else 0, position)
@@ -493,6 +508,8 @@ class StringLiteralValue(val value: String,
         this.parent = parent
     }
 
+    override val isSimple = true
+
     override fun replaceChildNode(node: Node, replacement: Node) {
         throw FatalAstException("can't replace here")
     }
@@ -523,6 +540,8 @@ class ArrayLiteralValue(val type: InferredTypes.InferredType,     // inferred be
         value.forEach {it.linkParents(this)}
     }
 
+    override val isSimple = true
+
     override fun replaceChildNode(node: Node, replacement: Node) {
         require(replacement is Expression)
         val idx = value.indexOfFirst { it===node }
@@ -629,6 +648,8 @@ class RangeExpr(var from: Expression,
         step.linkParents(this)
     }
 
+    override val isSimple = true
+
     override fun replaceChildNode(node: Node, replacement: Node) {
         require(replacement is Expression)
         when {
@@ -718,6 +739,8 @@ internal fun makeRange(fromVal: Int, toVal: Int, stepVal: Int): IntProgression {
 data class IdentifierReference(val nameInSource: List<String>, override val position: Position) : Expression(), IAssignable {
     override lateinit var parent: Node
 
+    override val isSimple = true
+
     fun targetStatement(program: Program) =
         if(nameInSource.size==1 && nameInSource[0] in program.builtinFunctions.names)
             BuiltinFunctionStatementPlaceholder(nameInSource[0], position, parent)
@@ -797,6 +820,8 @@ class FunctionCall(override var target: IdentifierReference,
         args.forEach { it.linkParents(this) }
     }
 
+    override val isSimple = target.nameInSource.size==1 && (target.nameInSource[0] in setOf("msb", "lsb", "peek", "peekw"))
+
     override fun replaceChildNode(node: Node, replacement: Node) {
         if(node===target)
             target=replacement as IdentifierReference

compilerAst/src/prog8/ast/statements/AstStatements.kt

@@ -662,7 +662,6 @@ class Subroutine(override val name: String,
     override lateinit var parent: Node
     val asmGenInfo = AsmGenInfo()
     val scopedname: String by lazy { makeScopedName(name) }
-    var inlineAnnotated: Boolean = false
 
     override fun linkParents(parent: Node) {
         this.parent = parent

docs/source/todo.rst

@@ -1,6 +1,13 @@
 ====
 TODO
 ====
+- make sure that in if statements, the left and right operand of the comparison is never a complex expression
+  anymore (only number, variable, addressof or memread) by rewriting if  <left> <op> <right>  {..} into:
+    if_eval_left = left
+    if_eval_right = right
+    if if_eval_left <op> if_eval_right { ... }          (only rewrite operand if x.isSimple() !!)
+  and then simplify all the codegeneration for if statements.
+  the variables are allocated in subroutine scope (otherwise irq handler could clobber them)
 
 - optimize several inner loops in gfx2
 - hoist all variable declarations up to the subroutine scope *before* even the constant folding takes place (to avoid undefined symbol errors when referring to a variable from another nested scope in the subroutine)

examples/test.p8

@@ -7,22 +7,6 @@ main {
         ubyte yy
         ubyte joy=1
         ubyte zz
-        str foobar="foobar"
-        uword joyw=1
-
-        if joyw + 1000 > 1000
-            txt.print(">1000")
-        if joyw + 1000 > 1011
-            txt.print(">1011")
-        if joyw + 1000 > & foobar
-            txt.print(">&foobar")
-        if joyw + 1000 > joyw
-            txt.print(">&foobar")
-
-        if joy + 10 > 10
-            txt.print(">10")
-        if joy + 10 >11
-            txt.print(">11")
 
         joy >>= 1
         if_cs
@@ -33,9 +17,18 @@ main {
             yy++
 
 ; TODO the shifting checks above result in way smaller code than this:
-         if joy & %00000001
+        if joy+44 > 33 {
             yy++
-         if joy & %00000010
+        }
+
+        yy=joy+44>33
+        if yy {
+            yy++
+        }
+
+        if joy & %00000001
+            yy++
+        if joy & %00000010
             yy++
 
     }