fix missing CMP in IR after conditional expression

codeGenIntermediate/src/prog8/codegen/intermediate/ExpressionGen.kt

@@ -1162,7 +1162,7 @@ internal class ExpressionGen(private val codeGen: IRCodeGen) {
             val resultRegister = codeGen.registers.next(IRDataType.BYTE)
             addInstr(result, IRInstruction(Opcode.FCOMP, IRDataType.FLOAT, reg1=resultRegister, fpReg1 = RegisterNum(leftTr.resultFpReg), fpReg2 = RegisterNum(rightTr.resultFpReg)), null)
             val branch = if(greaterEquals) Opcode.BGES else Opcode.BGTS
-            val resultReg = compareImmediateAsBooleanResult(branch, IRDataType.BYTE, resultRegister, 0, result)
+            val resultReg = loadBooleanStatusAsByte(branch, resultRegister, result)
             return ExpressionCodeResult(result, IRDataType.BYTE, resultReg, -1)
         } else {
             if(binExpr.left.type.isString || binExpr.right.type.isString) {
@@ -1198,7 +1198,7 @@ internal class ExpressionGen(private val codeGen: IRCodeGen) {
             val resultRegister = codeGen.registers.next(IRDataType.BYTE)
             addInstr(result, IRInstruction(Opcode.FCOMP, IRDataType.FLOAT, reg1=resultRegister, fpReg1 = RegisterNum(leftTr.resultFpReg), fpReg2 = RegisterNum(rightTr.resultFpReg)), null)
             val branch = if(lessEquals) Opcode.BLES else Opcode.BLTS
-            val resultReg = compareImmediateAsBooleanResult(branch, IRDataType.BYTE, resultRegister, 0, result)
+            val resultReg = loadBooleanStatusAsByte(branch, resultRegister, result)
             return ExpressionCodeResult(result, IRDataType.BYTE, resultReg, -1)
         } else {
             if(binExpr.left.type.isString || binExpr.right.type.isString) {
@@ -1239,7 +1239,11 @@ internal class ExpressionGen(private val codeGen: IRCodeGen) {
                 return if(rightConst!=null) {
                     val leftTr = translateExpression(binExpr.left)
                     addToResult(result, leftTr, leftTr.resultReg, -1)
-                    addInstr(result, IRInstruction(Opcode.CMPI, leftTr.dt, reg1 = leftTr.resultReg, immediate = rightConst.toInt()), null)
+                    val lastInstr = leftTr.chunks.lastOrNull()?.instructions?.lastOrNull()
+                    val skipCmpi = rightConst.toInt() == 0 && lastInstr != null && lastInstr.opcode in OpcodesThatSetStatusbits
+                    if (!skipCmpi) {
+                        addInstr(result, IRInstruction(Opcode.CMPI, leftTr.dt, reg1 = leftTr.resultReg, immediate = rightConst.toInt()), null)
+                    }
                     val resultReg = loadStatusAsBooleanResult(if(notEquals) Opcode.BSTNE else Opcode.BSTEQ, result)
                     ExpressionCodeResult(result, IRDataType.BYTE, resultReg, -1)
                 } else {
@@ -1287,12 +1291,12 @@ internal class ExpressionGen(private val codeGen: IRCodeGen) {
         return resultReg
     }
 
-    private fun compareImmediateAsBooleanResult(branchForTrue: Opcode, dt: IRDataType, reg1: Int, immediate: Int, result: MutableList<IRCodeChunkBase>): Int {
+    private fun loadBooleanStatusAsByte(branchForTrue: Opcode, reg1: Int, result: MutableList<IRCodeChunkBase>): Int {
         val labelTrue = codeGen.createLabelName()
         val labelDone = codeGen.createLabelName()
         val resultReg = codeGen.registers.next(IRDataType.BYTE)
         result += IRCodeChunk(null, null).also {
-            it += IRInstruction(branchForTrue, dt, reg1 = reg1, immediate = immediate, labelSymbol = labelTrue)
+            it += IRInstruction(branchForTrue, IRDataType.BYTE, reg1 = reg1, immediate = 0, labelSymbol = labelTrue)
             it += IRInstruction(Opcode.LOAD, IRDataType.BYTE, reg1 = resultReg, immediate = 0)
             it += IRInstruction(Opcode.JUMP, labelSymbol = labelDone)
         }

codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt

@@ -552,7 +552,7 @@ class IRCodeGen(
                             it += IRInstruction(Opcode.STOREM, irType(DataType.forDt(elementDt)), reg1=tmpReg, labelSymbol = loopvarSymbol)
                         }
                         result += translateNode(forLoop.statements)
-                        result += addConstIntToReg(IRDataType.BYTE, indexReg, elementSize)
+                        result += addConstByteToReg(indexReg, elementSize)
                         result += IRCodeChunk(null, null).also {
                             if(lengthBytes!=256) {
                                 // for length 256, the compare is actually against 0, which doesn't require a separate CMP instruction
@@ -710,29 +710,29 @@ class IRCodeGen(
         return result
     }
 
-    private fun addConstIntToReg(dt: IRDataType, reg: Int, value: Int): IRCodeChunk {
+    private fun addConstByteToReg(reg: Int, value: Int): IRCodeChunk {
         val code = IRCodeChunk(null, null)
         when(value) {
             0 -> { /* do nothing */ }
             1 -> {
-                code += IRInstruction(Opcode.INC, dt, reg1=reg)
+                code += IRInstruction(Opcode.INC, IRDataType.BYTE, reg1=reg)
             }
             2 -> {
-                code += IRInstruction(Opcode.INC, dt, reg1=reg)
-                code += IRInstruction(Opcode.INC, dt, reg1=reg)
+                code += IRInstruction(Opcode.INC, IRDataType.BYTE, reg1=reg)
+                code += IRInstruction(Opcode.INC, IRDataType.BYTE, reg1=reg)
             }
             -1 -> {
-                code += IRInstruction(Opcode.DEC, dt, reg1=reg)
+                code += IRInstruction(Opcode.DEC, IRDataType.BYTE, reg1=reg)
             }
             -2 -> {
-                code += IRInstruction(Opcode.DEC, dt, reg1=reg)
-                code += IRInstruction(Opcode.DEC, dt, reg1=reg)
+                code += IRInstruction(Opcode.DEC, IRDataType.BYTE, reg1=reg)
+                code += IRInstruction(Opcode.DEC, IRDataType.BYTE, reg1=reg)
             }
             else -> {
                 code += if(value>0) {
-                    IRInstruction(Opcode.ADD, dt, reg1 = reg, immediate = value)
+                    IRInstruction(Opcode.ADD, IRDataType.BYTE, reg1 = reg, immediate = value)
                 } else {
-                    IRInstruction(Opcode.SUB, dt, reg1 = reg, immediate = -value)
+                    IRInstruction(Opcode.SUB, IRDataType.BYTE, reg1 = reg, immediate = -value)
                 }
             }
         }
@@ -1068,19 +1068,6 @@ class IRCodeGen(
         return result
     }
 
-    private fun branchInstr(goto: PtJump, branchOpcode: Opcode): IRInstruction {
-        return if (goto.target.asConstInteger() != null)
-            IRInstruction(branchOpcode, address = goto.target.asConstInteger()?.toUInt()?.toAddress())
-        else {
-            require(!isIndirectJump(goto)) { "indirect jumps cannot be expressed using a branch opcode"}
-            val identifier = goto.target as? PtIdentifier
-            if(identifier!=null && !isIndirectJump(goto))
-                IRInstruction(branchOpcode, labelSymbol = identifier.name)
-            else
-                TODO("JUMP to expression address ${goto.target}  ${goto.position}")
-        }
-    }
-
     private fun translateCondition(
         condition: PtExpression,
         onTrueLabel: String?,
@@ -1127,6 +1114,13 @@ class IRCodeGen(
         // Fallback: materialize expression and branch on result
         val tr = expressionEval.translateExpression(condition)
         result += tr.chunks
+        // Check if the last instruction already sets status flags - if so, skip the CMPI #0
+        val lastInstr = tr.chunks.lastOrNull()?.instructions?.lastOrNull()
+        val skipCmpi = lastInstr != null && lastInstr.opcode in OpcodesThatSetStatusbits
+        if (!skipCmpi) {
+            addInstr(result, IRInstruction(Opcode.CMPI, tr.dt, reg1 = tr.resultReg, immediate = 0), null)
+        }
+
         if ((onTrueLabel != null || onTrueAddress != null) && (onFalseLabel != null || onFalseAddress != null)) {
             addInstr(result, IRInstruction(Opcode.BSTNE, labelSymbol = onTrueLabel, address = onTrueAddress), null)
             addInstr(result, IRInstruction(Opcode.JUMP, labelSymbol = onFalseLabel, address = onFalseAddress), null)
@@ -1161,14 +1155,20 @@ class IRCodeGen(
 
         if (number != null) {
             val isComparingWithZero = number == 0
+            val lastInstr = leftTr.chunks.lastOrNull()?.instructions?.lastOrNull()
+            val canSkipCmpi = isComparingWithZero && lastInstr != null && lastInstr.opcode in OpcodesThatSetStatusbits
+
             if ((onTrueLabel != null || onTrueAddress != null) && (onFalseLabel == null && onFalseAddress == null)) {
-                val (opcode, useCmpi) = getIntegerComparisonBranch(condition.operator, false, signed, isComparingWithZero)
+                var (opcode, useCmpi) = getIntegerComparisonBranch(condition.operator, false, signed)
+                if (canSkipCmpi && (opcode == Opcode.BSTEQ || opcode == Opcode.BSTNE)) useCmpi = false
                 emitIntegerComparisonBranch(result, opcode, useCmpi, branchDt, leftTr.resultReg, number, onTrueLabel, onTrueAddress)
             } else if ((onFalseLabel != null || onFalseAddress != null) && (onTrueLabel == null && onTrueAddress == null)) {
-                val (opcode, useCmpi) = getIntegerComparisonBranch(condition.operator, true, signed, isComparingWithZero)
+                var (opcode, useCmpi) = getIntegerComparisonBranch(condition.operator, true, signed)
+                if (canSkipCmpi && (opcode == Opcode.BSTEQ || opcode == Opcode.BSTNE)) useCmpi = false
                 emitIntegerComparisonBranch(result, opcode, useCmpi, branchDt, leftTr.resultReg, number, onFalseLabel, onFalseAddress)
             } else if ((onTrueLabel != null || onTrueAddress != null) && (onFalseLabel != null || onFalseAddress != null)) {
-                val (opcode, useCmpi) = getIntegerComparisonBranch(condition.operator, false, signed, isComparingWithZero)
+                var (opcode, useCmpi) = getIntegerComparisonBranch(condition.operator, false, signed)
+                if (canSkipCmpi && (opcode == Opcode.BSTEQ || opcode == Opcode.BSTNE)) useCmpi = false
                 emitIntegerComparisonBranch(result, opcode, useCmpi, branchDt, leftTr.resultReg, number, onTrueLabel, onTrueAddress)
                 addInstr(result, IRInstruction(Opcode.JUMP, labelSymbol = onFalseLabel, address = onFalseAddress), null)
             }
@@ -1228,7 +1228,7 @@ class IRCodeGen(
         else -> throw AssemblyError("invalid operator to invert")
     }
 
-    private fun getIntegerComparisonBranch(operator: String, invert: Boolean, signed: Boolean, isComparingWithZero: Boolean): Pair<Opcode, Boolean> {
+    private fun getIntegerComparisonBranch(operator: String, invert: Boolean, signed: Boolean): Pair<Opcode, Boolean> {
         val op = if (invert) invertOperator(operator) else operator
         return when (op) {
             "==" -> Opcode.BSTEQ to true

compiler/test/TestCompilerOnExamples.kt

@@ -336,17 +336,17 @@ class TestCompilerOnExamplesVirtual: FunSpec({
 
     val onlyVirtual = cartesianProduct(
         listOf(
-            ExampleSizes("bouncegfx", virtualInstrCountOptimized=186, virtualInstrCountUnoptimized=834, virtualRegCountOptimized=92, virtualRegCountUnoptimized=380),
-            ExampleSizes("bsieve", virtualInstrCountOptimized=158, virtualInstrCountUnoptimized=1953, virtualRegCountOptimized=66, virtualRegCountUnoptimized=860),
-            ExampleSizes("fountain", virtualInstrCountOptimized=152, virtualInstrCountUnoptimized=802, virtualRegCountOptimized=77, virtualRegCountUnoptimized=363),
+            ExampleSizes("bouncegfx", virtualInstrCountOptimized=186, virtualInstrCountUnoptimized=824, virtualRegCountOptimized=92, virtualRegCountUnoptimized=380),
+            ExampleSizes("bsieve", virtualInstrCountOptimized=158, virtualInstrCountUnoptimized=1939, virtualRegCountOptimized=66, virtualRegCountUnoptimized=860),
+            ExampleSizes("fountain", virtualInstrCountOptimized=152, virtualInstrCountUnoptimized=791, virtualRegCountOptimized=77, virtualRegCountUnoptimized=363),
             ExampleSizes("pixelshader", virtualInstrCountOptimized=43, virtualInstrCountUnoptimized=291, virtualRegCountOptimized=15, virtualRegCountUnoptimized=109),
-            ExampleSizes("sincos", virtualInstrCountOptimized=276, virtualInstrCountUnoptimized=908, virtualRegCountOptimized=133, virtualRegCountUnoptimized=407),
-            ExampleSizes("pointers/animalgame", virtualInstrCountOptimized=221, virtualInstrCountUnoptimized=2097, virtualRegCountOptimized=103, virtualRegCountUnoptimized=933),
-            ExampleSizes("pointers/binarytree", virtualInstrCountOptimized=510, virtualInstrCountUnoptimized=2268, virtualRegCountOptimized=251, virtualRegCountUnoptimized=1020),
-            ExampleSizes("pointers/hashtable", virtualInstrCountOptimized=578, virtualInstrCountUnoptimized=2279, virtualRegCountOptimized=293, virtualRegCountUnoptimized=1045),
-            ExampleSizes("pointers/sortedlist", virtualInstrCountOptimized=267, virtualInstrCountUnoptimized=2434, virtualRegCountOptimized=118, virtualRegCountUnoptimized=1086),
-            ExampleSizes("pointers/fountain-virtual", virtualInstrCountOptimized=146, virtualInstrCountUnoptimized=797, virtualRegCountOptimized=71, virtualRegCountUnoptimized=357),
-            ExampleSizes("pointers/sorting", virtualInstrCountOptimized=369, virtualInstrCountUnoptimized=2266, virtualRegCountOptimized=173, virtualRegCountUnoptimized=1010)
+            ExampleSizes("sincos", virtualInstrCountOptimized=276, virtualInstrCountUnoptimized=898, virtualRegCountOptimized=133, virtualRegCountUnoptimized=407),
+            ExampleSizes("pointers/animalgame", virtualInstrCountOptimized=221, virtualInstrCountUnoptimized=2083, virtualRegCountOptimized=103, virtualRegCountUnoptimized=933),
+            ExampleSizes("pointers/binarytree", virtualInstrCountOptimized=510, virtualInstrCountUnoptimized=2237, virtualRegCountOptimized=251, virtualRegCountUnoptimized=1020),
+            ExampleSizes("pointers/hashtable", virtualInstrCountOptimized=578, virtualInstrCountUnoptimized=2255, virtualRegCountOptimized=293, virtualRegCountUnoptimized=1045),
+            ExampleSizes("pointers/sortedlist", virtualInstrCountOptimized=267, virtualInstrCountUnoptimized=2406, virtualRegCountOptimized=118, virtualRegCountUnoptimized=1086),
+            ExampleSizes("pointers/fountain-virtual", virtualInstrCountOptimized=146, virtualInstrCountUnoptimized=785, virtualRegCountOptimized=71, virtualRegCountUnoptimized=357),
+            ExampleSizes("pointers/sorting", virtualInstrCountOptimized=369, virtualInstrCountUnoptimized=2251, virtualRegCountOptimized=173, virtualRegCountUnoptimized=1010)
         ),
         listOf(false, true)
     )

compiler/test/vm/TestCompilerVirtual.kt

@@ -420,11 +420,11 @@ main {
         nextInstr.opcode shouldBeIn setOf(Opcode.BSTEQ, Opcode.BSTNE)
     }
 
-    test("if-expression keeps CMPI when condition doesn't set flags") {
+    test("if-expression omits CMPI since CALL now sets flags") {
         val src = """
 main {
+    sub getflag() -> bool { return true }
     sub start() {
-        sub getflag() -> bool { return true }
         cx16.r0L = if getflag() then 1 else 0
     }
 }"""
@@ -434,9 +434,9 @@ main {
         val start = irProgram.blocks[0].children[0] as IRSubroutine
         val instructions = start.chunks.flatMap { c->c.instructions }
 
-        // Should contain CMPI #0 after function call (since CALL doesn't set flags)
+        // Should NOT contain CMPI #0 after function call anymore (since CALL effectively sets flags now)
         val cmpiInstr = instructions.find { it.opcode == Opcode.CMPI && it.immediate == 0 }
-        cmpiInstr shouldNotBe null
+        cmpiInstr shouldBe null
     }
 
     test("repeat counts (const)") {

intermediate/src/prog8/intermediate/IRInstructions.kt

@@ -533,6 +533,7 @@ val OpcodesThatSetStatusbitsIncludingCarry = setOf(
     Opcode.SGN
 )
 
+// Note: never include CALL, SYSCALL and CONCAT here because they should never be assumed to have set sensible status bits.
 val OpcodesThatSetStatusbitsButNotCarry = setOf(
     Opcode.LOAD,
     Opcode.LOADM,
@@ -565,8 +566,7 @@ val OpcodesThatSetStatusbitsButNotCarry = setOf(
     Opcode.MSIGB,
     Opcode.MSIGW,
     Opcode.BSIGB,
-    Opcode.MIDB,
-    Opcode.CONCAT
+    Opcode.MIDB
 )
 
 val OpcodesThatDependOnCarry = setOf(

virtualmachine/src/prog8/vm/VirtualMachine.kt

@@ -688,27 +688,30 @@ class VirtualMachine(irProgram: IRProgram) {
             val returns = context.fcallSpec.returns
             when (i.type!!) {
                 IRDataType.BYTE -> {
-                    if(returns.isNotEmpty())
-                        registers.setUB(returns.single().registerNum, i.immediate!!.toUByte())
-                    else {
+                    if(returns.isNotEmpty()) {
+                        val value = i.immediate!!.toUByte()
+                        registers.setUB(returns.single().registerNum, value)
+                    } else {
                         val callInstr = context.returnChunk.instructions[context.returnIndex-1]
                         if(callInstr.opcode!=Opcode.CALL)
                             throw IllegalArgumentException("missing return value reg")
                     }
                 }
                 IRDataType.WORD -> {
-                    if(returns.isNotEmpty())
-                        registers.setUW(returns.single().registerNum, i.immediate!!.toUShort())
-                    else {
+                    if(returns.isNotEmpty()) {
+                        val value = i.immediate!!.toUShort()
+                        registers.setUW(returns.single().registerNum, value)
+                    } else {
                         val callInstr = context.returnChunk.instructions[context.returnIndex-1]
                         if(callInstr.opcode!=Opcode.CALL)
                             throw IllegalArgumentException("missing return value reg")
                     }
                 }
                 IRDataType.LONG -> {
-                    if(returns.isNotEmpty())
-                        registers.setSL(returns.single().registerNum, i.immediate!!)
-                    else {
+                    if(returns.isNotEmpty()) {
+                        val value = i.immediate!!
+                        registers.setSL(returns.single().registerNum, value)
+                    } else {
                         val callInstr = context.returnChunk.instructions[context.returnIndex-1]
                         if(callInstr.opcode!=Opcode.CALL)
                             throw IllegalArgumentException("missing return value reg")
@@ -737,27 +740,30 @@ class VirtualMachine(irProgram: IRProgram) {
             val returns = context.fcallSpec.returns
             when (i.type!!) {
                 IRDataType.BYTE -> {
-                    if(returns.isNotEmpty())
-                        registers.setUB(returns.single().registerNum, registers.getUB(i.reg1!!))
-                    else {
+                    if(returns.isNotEmpty()) {
+                        val value = registers.getUB(i.reg1!!)
+                        registers.setUB(returns.single().registerNum, value)
+                    } else {
                         val callInstr = context.returnChunk.instructions[context.returnIndex-1]
                         if(callInstr.opcode!=Opcode.CALL)
                             throw IllegalArgumentException("missing return value reg")
                     }
                 }
                 IRDataType.WORD -> {
-                    if(returns.isNotEmpty())
-                        registers.setUW(returns.single().registerNum, registers.getUW(i.reg1!!))
-                    else {
+                    if(returns.isNotEmpty()) {
+                        val value = registers.getUW(i.reg1!!)
+                        registers.setUW(returns.single().registerNum, value)
+                    } else {
                         val callInstr = context.returnChunk.instructions[context.returnIndex-1]
                         if(callInstr.opcode!=Opcode.CALL)
                             throw IllegalArgumentException("missing return value reg")
                     }
                 }
                 IRDataType.LONG -> {
-                    if(returns.isNotEmpty())
-                        registers.setSL(returns.single().registerNum, registers.getSL(i.reg1!!))
-                    else {
+                    if(returns.isNotEmpty()) {
+                        val value = registers.getSL(i.reg1!!)
+                        registers.setSL(returns.single().registerNum, value)
+                    } else {
                         val callInstr = context.returnChunk.instructions[context.returnIndex-1]
                         if(callInstr.opcode!=Opcode.CALL)
                             throw IllegalArgumentException("missing return value reg")
@@ -1537,7 +1543,7 @@ class VirtualMachine(irProgram: IRProgram) {
     }
 
 
-    private fun statusbitsNZ(value: Int, type: IRDataType) {
+    internal fun statusbitsNZ(value: Int, type: IRDataType) {
         statusZero = value == 0
         when(type) {
             IRDataType.BYTE -> statusNegative = (value and 0x80) == 0x80
@@ -2723,7 +2729,6 @@ class VirtualMachine(irProgram: IRProgram) {
                 val lsb = registers.getUB(i.reg3!!)
                 val value = ((msb.toInt() shl 8) or lsb.toInt())
                 registers.setUW(i.reg1!!, value.toUShort())
-                statusbitsNZ(value, i.type!!)
             }
             IRDataType.WORD -> {
                 val msw = registers.getUW(i.reg2!!)