don't swap operands that would change function evaluation order + vm: fix label casing error

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

@@ -291,24 +291,7 @@ internal class AssignmentAsmGen(private val program: Program,
                 assignRegisterByte(assign.target, CpuRegister.A)
             }
             is BinaryExpression -> {
-                if(value.operator in ComparisonOperators) {
+                if(!attemptAssignOptimizedBinexpr(value, assign)) {
-                    // TODO real optimized code for comparison expressions that yield a boolean result value
-                    assignConstantByte(assign.target, 0)
-                    val origTarget = assign.target.origAstTarget
-                    if(origTarget!=null) {
-                        val assignTrue = AnonymousScope(mutableListOf(
-                            Assignment(origTarget, NumericLiteral.fromBoolean(true, assign.position), AssignmentOrigin.ASMGEN, assign.position)
-                        ), assign.position)
-                        val assignFalse = AnonymousScope(mutableListOf(), assign.position)
-                        val ifelse = IfElse(value.copy(), assignTrue, assignFalse, assign.position)
-                        ifelse.linkParents(value)
-                        asmgen.translate(ifelse)
-                    }
-                    else {
-                        // no orig ast assign target so can't use the workaround, so fallback to stack eval
-                        fallbackToStackEval(assign)
-                    }
-                } else if(!attemptAssignOptimizedBinexpr(value, assign)) {
                     // All remaining binary expressions just evaluate via the stack for now.
                     // (we can't use the assignment helper functions (assignExpressionTo...) to do it via registers here,
                     // because the code here is the implementation of exactly that...)
@@ -320,8 +303,78 @@ internal class AssignmentAsmGen(private val program: Program,
     }
 
     private fun attemptAssignOptimizedBinexpr(expr: BinaryExpression, assign: AsmAssignment): Boolean {
+        if(expr.operator in ComparisonOperators) {
+            assignConstantByte(assign.target, 0)
+            val origTarget = assign.target.origAstTarget
+            if(origTarget!=null) {
+                val assignTrue = AnonymousScope(mutableListOf(
+                    Assignment(origTarget, NumericLiteral.fromBoolean(true, assign.position), AssignmentOrigin.ASMGEN, assign.position)
+                ), assign.position)
+                val assignFalse = AnonymousScope(mutableListOf(), assign.position)
+                val ifelse = IfElse(expr.copy(), assignTrue, assignFalse, assign.position)
+                ifelse.linkParents(expr)
+                asmgen.translate(ifelse)
+                return true
+            }
+        }
+
         if(!expr.inferType(program).isInteger)
             return false
+
+/* TODO re-add these optimizations? after we improved the unneeded addition of !=0 expressions
+        if(expr.operator=="and") {
+            val dt = expr.left.inferType(program).getOrElse { throw AssemblyError("weird dt") }
+            if (dt in ByteDatatypes) {
+                assignExpressionToRegister(expr.left, RegisterOrPair.A, dt==DataType.BYTE || dt==DataType.WORD)
+                asmgen.out("  pha")
+                assignExpressionToVariable(expr.right, "P8ZP_SCRATCH_B1", dt, expr.definingSubroutine)
+                asmgen.out("  pla |  and  P8ZP_SCRATCH_B1")
+                if(assign.target.datatype in ByteDatatypes)
+                    assignRegisterByte(assign.target, CpuRegister.A)
+                else {
+                    asmgen.out("  ldy  #0")
+                    assignRegisterpairWord(assign.target, RegisterOrPair.AY)
+                }
+                return true
+            }
+            else throw AssemblyError("weird dt for and, expected byte $expr @${expr.position}")
+        }
+        else if(expr.operator=="or") {
+            val dt = expr.left.inferType(program).getOrElse { throw AssemblyError("weird dt") }
+            if (dt in ByteDatatypes) {
+                assignExpressionToRegister(expr.left, RegisterOrPair.A, dt==DataType.BYTE || dt==DataType.WORD)
+                asmgen.out("  pha")
+                assignExpressionToVariable(expr.right, "P8ZP_SCRATCH_B1", dt, expr.definingSubroutine)
+                asmgen.out("  pla |  ora  P8ZP_SCRATCH_B1")
+                if(assign.target.datatype in ByteDatatypes)
+                    assignRegisterByte(assign.target, CpuRegister.A)
+                else {
+                    asmgen.out("  ldy  #0")
+                    assignRegisterpairWord(assign.target, RegisterOrPair.AY)
+                }
+                return true
+            }
+            else throw AssemblyError("weird dt for or, expected byte $expr @${expr.position}")
+        }
+        else if(expr.operator=="xor") {
+            val dt = expr.left.inferType(program).getOrElse { throw AssemblyError("weird dt") }
+            if (dt in ByteDatatypes) {
+                assignExpressionToRegister(expr.left, RegisterOrPair.A, dt==DataType.BYTE || dt==DataType.WORD)
+                asmgen.out("  pha")
+                assignExpressionToVariable(expr.right, "P8ZP_SCRATCH_B1", dt, expr.definingSubroutine)
+                asmgen.out("  pla |  eor  P8ZP_SCRATCH_B1")
+                if(assign.target.datatype in ByteDatatypes)
+                    assignRegisterByte(assign.target, CpuRegister.A)
+                else {
+                    asmgen.out("  ldy  #0")
+                    assignRegisterpairWord(assign.target, RegisterOrPair.AY)
+                }
+                return true
+            }
+            else throw AssemblyError("weird dt for xor, expected byte $expr @${expr.position}")
+        }
+*/
+
         if(expr.operator!="+" && expr.operator!="-")
             return false
 

codeOptimizers/src/prog8/optimizer/ConstantFoldingOptimizer.kt

@@ -6,6 +6,7 @@ import prog8.ast.base.ExpressionError
 import prog8.ast.base.FatalAstException
 import prog8.ast.base.UndefinedSymbolError
 import prog8.ast.expressions.*
+import prog8.ast.maySwapOperandOrder
 import prog8.ast.statements.ForLoop
 import prog8.ast.statements.VarDecl
 import prog8.ast.statements.VarDeclType
@@ -438,7 +439,7 @@ class ConstantFoldingOptimizer(private val program: Program) : AstWalker() {
             // both operators are the same.
 
             // If associative,  we can simply shuffle the const operands around to optimize.
-            if(expr.operator in AssociativeOperators) {
+            if(expr.operator in AssociativeOperators && maySwapOperandOrder(expr)) {
                 return if(leftIsConst) {
                     if(subleftIsConst)
                         ShuffleOperands(expr, null, subExpr, subExpr.right, null, null, expr.left)

codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt

@@ -5,6 +5,7 @@ import prog8.ast.Node
 import prog8.ast.Program
 import prog8.ast.base.FatalAstException
 import prog8.ast.expressions.*
+import prog8.ast.maySwapOperandOrder
 import prog8.ast.statements.AnonymousScope
 import prog8.ast.statements.Assignment
 import prog8.ast.statements.IfElse
@@ -88,12 +89,12 @@ class ExpressionSimplifier(private val program: Program) : AstWalker() {
             throw FatalAstException("can't determine datatype of both expression operands $expr")
 
         // ConstValue <associativeoperator> X -->  X <associativeoperator> ConstValue
-        if (leftVal != null && expr.operator in AssociativeOperators && rightVal == null)
+        if (leftVal != null && expr.operator in AssociativeOperators && rightVal == null && maySwapOperandOrder(expr))
             return listOf(IAstModification.SwapOperands(expr))
 
         // NonBinaryExpression  <associativeoperator>  BinaryExpression  -->  BinaryExpression  <associativeoperator>  NonBinaryExpression
         if (expr.operator in AssociativeOperators && expr.left !is BinaryExpression && expr.right is BinaryExpression) {
-            if(parent !is Assignment || !(expr.left isSameAs parent.target))
+            if(parent !is Assignment || !(expr.left isSameAs parent.target) && maySwapOperandOrder(expr))
                 return listOf(IAstModification.SwapOperands(expr))
         }
 
@@ -630,7 +631,7 @@ class ExpressionSimplifier(private val program: Program) : AstWalker() {
     }
 
     private fun reorderAssociativeWithConstant(expr: BinaryExpression, leftVal: NumericLiteral?): BinExprWithConstants {
-        if (expr.operator in AssociativeOperators && leftVal != null) {
+        if (expr.operator in AssociativeOperators && leftVal != null && maySwapOperandOrder(expr)) {
             // swap left and right so that right is always the constant
             val tmp = expr.left
             expr.left = expr.right

codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt

@@ -273,7 +273,7 @@ class StatementOptimizer(private val program: Program,
                     val op1 = binExpr.operator
                     val op2 = rExpr.operator
 
-                    if(rExpr.left is NumericLiteral && op2 in AssociativeOperators) {
+                    if(rExpr.left is NumericLiteral && op2 in AssociativeOperators && maySwapOperandOrder(binExpr)) {
                         // associative operator, make sure the constant numeric value is second (right)
                         return listOf(IAstModification.SwapOperands(rExpr))
                     }
@@ -312,7 +312,7 @@ class StatementOptimizer(private val program: Program,
             if(binExpr.operator in AssociativeOperators && binExpr.right isSameAs assignment.target) {
                 // associative operator, swap the operands so that the assignment target is first (left)
                 // unless the other operand is the same in which case we don't swap (endless loop!)
-                if (!(binExpr.left isSameAs binExpr.right))
+                if (!(binExpr.left isSameAs binExpr.right) && maySwapOperandOrder(binExpr))
                     return listOf(IAstModification.SwapOperands(binExpr))
             }
 

compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt

@@ -194,7 +194,10 @@ internal class StatementReorderer(val program: Program,
         // ConstValue <associativeoperator> X -->  X <associativeoperator> ConstValue
         // (this should be done by the ExpressionSimplifier when optimizing is enabled,
         //  but the current assembly code generator for IF statements now also depends on it, so we do it here regardless of optimization.)
-        if (expr.left.constValue(program) != null && expr.operator in AssociativeOperators && expr.right.constValue(program) == null)
+        if (expr.left.constValue(program) != null
+            && expr.operator in AssociativeOperators
+            && expr.right.constValue(program) == null
+            && maySwapOperandOrder(expr))
             return listOf(IAstModification.SwapOperands(expr))
 
         // when using a simple bit shift and assigning it to a variable of a different type,
@@ -322,7 +325,7 @@ internal class StatementReorderer(val program: Program,
                 return noModifications
             }
 
-            if(binExpr.operator in AssociativeOperators) {
+            if(binExpr.operator in AssociativeOperators && maySwapOperandOrder(binExpr)) {
                 if (binExpr.right isSameAs assignment.target) {
                     // A = v <associative-operator> A  ==>  A = A <associative-operator> v
                     return listOf(IAstModification.SwapOperands(binExpr))

compilerAst/src/prog8/ast/Extensions.kt

@@ -1,9 +1,7 @@
 package prog8.ast
 
 import prog8.ast.base.FatalAstException
-import prog8.ast.expressions.Expression
+import prog8.ast.expressions.*
-import prog8.ast.expressions.IdentifierReference
-import prog8.ast.expressions.InferredTypes
 import prog8.ast.statements.*
 import prog8.code.core.DataType
 import prog8.code.core.Position
@@ -86,3 +84,14 @@ fun determineGosubArguments(gosub: GoSub): Map<String, Expression> {
     }
     return arguments
 }
+
+fun maySwapOperandOrder(binexpr: BinaryExpression): Boolean {
+    fun ok(expr: Expression): Boolean {
+        return when(expr) {
+            is BinaryExpression -> expr.left.isSimple
+            is IFunctionCall -> false
+            else -> expr.isSimple
+        }
+    }
+    return ok(binexpr.left) || ok(binexpr.right)
+}

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

@@ -141,7 +141,7 @@ class PrefixExpression(val operator: String, var expression: Expression, overrid
         }
     }
 
-    override val isSimple = true
+    override val isSimple = expression.isSimple
 
     override fun toString(): String {
         return "Prefix($operator $expression)"

docs/source/todo.rst

@@ -3,6 +3,8 @@ TODO
 
 For next release
 ^^^^^^^^^^^^^^^^
+- get rid of unneeded !=0 added to logical expressions
+- optimize logical expressions in attemptAssignOptimizedBinexpr()
 - add McCarthy evaluation to shortcircuit and/or expressions. First do ifs by splitting them up? Then do expressions that compute a value?
 ...
 

examples/test.p8

@@ -29,23 +29,92 @@ main  {
 ;    }
 
 
-    sub func1(ubyte a1, ubyte a2) -> ubyte {
+    sub funcFalse() -> ubyte {
-        return a1+a2+42
+        txt.print("false() ")
+        return false
     }
 
-    sub func2(ubyte a1, ubyte a2) -> ubyte {
+    sub funcFalseWord() -> uword {
-        return 200-a1-a2
+        txt.print("falseWord() ")
+        return 0
+    }
+
+    sub funcTrue() -> ubyte {
+        txt.print("ftrue() ")
+        return true
+    }
+
+    sub func1(ubyte a1) -> ubyte {
+        txt.print("func1() ")
+        return a1>1
+    }
+
+    sub func2(ubyte a1) -> ubyte {
+        txt.print("func2() ")
+        return a1>2
+    }
+
+    sub func3(ubyte a1) -> ubyte {
+        txt.print("func3() ")
+        return a1>3
+    }
+
+    sub func4(ubyte a1) -> ubyte {
+        txt.print("func4() ")
+        return a1>4
+    }
+
+    sub funcw() -> uword {
+        txt.print("funcw() ")
+        return 9999
     }
 
     sub start() {
         ; mcCarthy()
+        ubyte value
+        uword wvalue
 
-        ; 9+10+42 = 61
+        txt.print("short and with false (word): ")
-        ; 200-61-2 = 137
+        wvalue = funcw() and funcFalseWord() and funcw() and funcw()
-        ; ubyte result = 9 |> func1(10) |> func2(2)
+        txt.print_uw(wvalue)
-        ; $090a $0a02
+        txt.nl()
-        uword resultw = 9 |> mkword(10) |> lsb() |> mkword(2)
+
-        txt.print_uw(resultw)
+        txt.print("short and with false: ")
+        value = func1(25) and funcFalse()
+        txt.print_ub(value)
+        txt.nl()
+        txt.print("short or with true: ")
+        value = func1(25) or funcTrue()
+        txt.print_ub(value)
+        txt.nl()
+        txt.print("short xor with false: ")
+        value = func1(25) xor funcFalse()
+        txt.print_ub(value)
+        txt.nl()
+
+        txt.print("and with false: ")
+        value = func1(25) and func2(25) and funcFalse() and func3(25) and func4(25)
+        txt.print_ub(value)
+        txt.nl()
+        txt.print("and with true:  ")
+        value = func1(25) and func2(25) and funcTrue() and func3(25) and func4(25)
+        txt.print_ub(value)
+        txt.nl()
+        txt.print("or with false:  ")
+        value = func1(25) or func2(25) or funcFalse() or func3(25) or func4(25)
+        txt.print_ub(value)
+        txt.nl()
+        txt.print("or with true:   ")
+        value = func1(25) or func2(25) or funcTrue() or func3(25) or func4(25)
+        txt.print_ub(value)
+        txt.nl()
+        txt.print("xor with false: ")
+        value = func1(25) xor func2(25) xor funcFalse() xor func3(25) xor func4(25)
+        txt.print_ub(value)
+        txt.nl()
+        txt.print("xor with true:  ")
+        value = func1(25) xor func2(25) xor funcTrue() xor func3(25) xor func4(25)
+        txt.print_ub(value)
         txt.nl()
 
         ; a "pixelshader":

syntax-files/Vim/prog8.vim

@@ -24,7 +24,6 @@ syn match prog8Function "\(\<\(asm\)\?sub\>\s\+\)\@16<=\<\w\+\>"
 syn match prog8Function "\(romsub\s\+$\x\+\s\+=\s\+\)\@16<=\<\w\+\>"
 
 syn keyword prog8Statement break goto return asmsub sub inline
-syn match prog8Statement "|>"
 syn match prog8Statement "\<\(asm\|rom\)\?sub\>"
 syn keyword prog8Conditional if else when
 syn keyword prog8Conditional if_cs if_cc if_vs if_vc if_eq if_z if_ne if_nz

virtualmachine/src/prog8/vm/Assembler.kt

@@ -65,7 +65,7 @@ class Assembler {
         placeholders.clear()
         val program = mutableListOf<Instruction>()
         val instructionPattern = Regex("""([a-z]+)(\.b|\.w|\.f)?(.*)""", RegexOption.IGNORE_CASE)
-        val labelPattern = Regex("""_([a-z\d\._]+):""")
+        val labelPattern = Regex("""_([a-zA-Z\d\._]+):""")
         for (line in source.lines()) {
             if(line.isBlank() || line.startsWith(';'))
                 continue
@@ -116,7 +116,13 @@ class Assembler {
                     else if(operand[0]=='f' && operand[1]=='r')
                         fpReg1 = operand.substring(2).toInt()
                     else {
-                        value = parseValue(operand, program.size)
+                        value = if(operand.startsWith('_')) {
+                            // it's a label, keep the original case!
+                            val labelname = rest.split(",").first().trim()
+                            parseValue(labelname, program.size)
+                        } else {
+                            parseValue(operand, program.size)
+                        }
                         operands.clear()
                     }
                     if(operands.isNotEmpty()) {