fix expression evaluation bug where intermediate values were overwritten, yielding the wrong result

This commit is contained in:
Irmen de Jong 2024-01-04 20:44:46 +01:00
parent 56ba24962c
commit bfd3edb617
5 changed files with 43 additions and 30 deletions

View File

@ -79,7 +79,12 @@ sealed class PtExpression(val type: DataType, position: Position) : PtNode(posit
is PtArray -> true
is PtArrayIndexer -> index is PtNumber || index is PtIdentifier
is PtBinaryExpression -> false
is PtBuiltinFunctionCall -> name in arrayOf("msb", "lsb", "peek", "peekw", "mkword", "set_carry", "set_irqd", "clear_carry", "clear_irqd")
is PtBuiltinFunctionCall -> {
when (name) {
in arrayOf("msb", "lsb", "mkword", "set_carry", "set_irqd", "clear_carry", "clear_irqd") -> this.args.all { it.isSimple() }
else -> false
}
}
is PtContainmentCheck -> false
is PtFunctionCall -> false
is PtIdentifier -> true

View File

@ -563,11 +563,11 @@ internal class AssignmentAsmGen(private val program: PtProgram,
asmgen.assignExpressionToRegister(expr.right, RegisterOrPair.R1, expr.left.type in SignedDatatypes)
} else {
asmgen.assignExpressionToRegister(expr.left, RegisterOrPair.AY, expr.left.type in SignedDatatypes)
asmgen.saveRegisterStack(CpuRegister.A, false)
asmgen.out(" pha")
asmgen.saveRegisterStack(CpuRegister.Y, false)
asmgen.assignExpressionToRegister(expr.right, RegisterOrPair.R1, expr.left.type in SignedDatatypes)
asmgen.restoreRegisterStack(CpuRegister.Y, false)
asmgen.restoreRegisterStack(CpuRegister.A, false)
asmgen.out(" pla")
asmgen.out(" sta cx16.r0 | sty cx16.r0+1")
}
asmgen.out(" jsr verafx.muls")
@ -813,8 +813,10 @@ internal class AssignmentAsmGen(private val program: PtProgram,
}
assignRegisterByte(target, CpuRegister.A, dt in SignedDatatypes, true)
} else {
assignExpressionToVariable(right, "P8ZP_SCRATCH_B1", right.type)
assignExpressionToRegister(left, RegisterOrPair.A, left.type==DataType.BYTE)
asmgen.out(" pha")
assignExpressionToVariable(right, "P8ZP_SCRATCH_B1", right.type)
asmgen.out(" pla")
if (expr.operator == "+")
asmgen.out(" clc | adc P8ZP_SCRATCH_B1")
else
@ -1003,9 +1005,9 @@ internal class AssignmentAsmGen(private val program: PtProgram,
}
assignExpressionToRegister(expr.left, RegisterOrPair.A, false)
asmgen.saveRegisterStack(CpuRegister.A, false)
asmgen.out(" pha")
assignExpressionToVariable(expr.right, "P8ZP_SCRATCH_B1", DataType.UBYTE)
asmgen.restoreRegisterStack(CpuRegister.A, false)
asmgen.out(" pla")
when (expr.operator) {
"&" -> asmgen.out(" and P8ZP_SCRATCH_B1")
"|" -> asmgen.out(" ora P8ZP_SCRATCH_B1")
@ -1073,9 +1075,9 @@ internal class AssignmentAsmGen(private val program: PtProgram,
} else {
// normal evaluation into A
assignExpressionToRegister(expr.left, RegisterOrPair.A, false)
asmgen.saveRegisterStack(CpuRegister.A, false)
asmgen.out(" pha")
assignExpressionToVariable(expr.right, "P8ZP_SCRATCH_B1", DataType.UBYTE)
asmgen.restoreRegisterStack(CpuRegister.A, false)
asmgen.out(" pla")
when (expr.operator) {
"and" -> asmgen.out(" and P8ZP_SCRATCH_B1")
"or" -> asmgen.out(" ora P8ZP_SCRATCH_B1")
@ -1780,8 +1782,10 @@ internal class AssignmentAsmGen(private val program: PtProgram,
private fun assignLogicalAndOrWithSimpleRightOperandByte(target: AsmAssignTarget, left: PtExpression, operator: String, right: PtExpression) {
// normal evaluation, not worth to shortcircuit the simple right operand
assignExpressionToVariable(left, "P8ZP_SCRATCH_B1", DataType.UBYTE)
assignExpressionToRegister(right, RegisterOrPair.A, false)
assignExpressionToRegister(left, RegisterOrPair.A, false)
asmgen.out(" pha")
assignExpressionToVariable(right, "P8ZP_SCRATCH_B1", DataType.UBYTE)
asmgen.out(" pla")
when (operator) {
"and" -> asmgen.out(" and P8ZP_SCRATCH_B1")
"or" -> asmgen.out(" ora P8ZP_SCRATCH_B1")
@ -1896,9 +1900,9 @@ internal class AssignmentAsmGen(private val program: PtProgram,
DataType.STR -> {
// use subroutine
assignExpressionToRegister(containment.element, RegisterOrPair.A, elementDt == DataType.BYTE)
asmgen.saveRegisterStack(CpuRegister.A, true)
asmgen.out(" pha")
assignAddressOf(AsmAssignTarget(TargetStorageKind.VARIABLE, asmgen, DataType.UWORD, containment.definingISub(), containment.position,"P8ZP_SCRATCH_W1"), symbolName, null, null)
asmgen.restoreRegisterStack(CpuRegister.A, false)
asmgen.out(" pla")
asmgen.out(" ldy #${numElements-1}")
asmgen.out(" jsr prog8_lib.containment_bytearray")
return
@ -1908,9 +1912,9 @@ internal class AssignmentAsmGen(private val program: PtProgram,
}
DataType.ARRAY_B, DataType.ARRAY_UB -> {
assignExpressionToRegister(containment.element, RegisterOrPair.A, elementDt == DataType.BYTE)
asmgen.saveRegisterStack(CpuRegister.A, true)
asmgen.out(" pha")
assignAddressOf(AsmAssignTarget(TargetStorageKind.VARIABLE, asmgen, DataType.UWORD, containment.definingISub(), containment.position, "P8ZP_SCRATCH_W1"), symbolName, null, null)
asmgen.restoreRegisterStack(CpuRegister.A, false)
asmgen.out(" pla")
asmgen.out(" ldy #$numElements")
asmgen.out(" jsr prog8_lib.containment_bytearray")
return
@ -2725,11 +2729,11 @@ internal class AssignmentAsmGen(private val program: PtProgram,
jsr floats.copy_float""")
}
TargetStorageKind.ARRAY -> {
asmgen.saveRegisterStack(CpuRegister.A, false)
asmgen.out(" pha")
asmgen.saveRegisterStack(CpuRegister.Y, false)
asmgen.assignExpressionToRegister(target.array!!.index, RegisterOrPair.A, false)
asmgen.restoreRegisterStack(CpuRegister.Y, false)
asmgen.restoreRegisterStack(CpuRegister.A, false)
asmgen.out(" pla")
asmgen.out("""
sta P8ZP_SCRATCH_W1
sty P8ZP_SCRATCH_W1+1
@ -3261,7 +3265,7 @@ internal class AssignmentAsmGen(private val program: PtProgram,
} else {
asmgen.saveRegisterStack(register, false)
asmgen.assignExpressionToRegister(target.array.index, RegisterOrPair.Y, false)
asmgen.restoreRegisterStack(CpuRegister.A, false)
asmgen.out(" pla")
if(asmgen.isZpVar(target.origAstTarget!!.array!!.variable)) {
asmgen.out(" sta (${target.asmVarname}),y")
} else {
@ -3298,8 +3302,7 @@ internal class AssignmentAsmGen(private val program: PtProgram,
require(target.array.index.type in ByteDatatypes)
asmgen.saveRegisterStack(register, false)
asmgen.assignExpressionToRegister(target.array.index, RegisterOrPair.Y, false)
asmgen.restoreRegisterStack(CpuRegister.A, false)
asmgen.out(" sta ${target.asmVarname},y")
asmgen.out(" pla | sta ${target.asmVarname},y")
}
}
}

View File

@ -1228,7 +1228,10 @@ class BuiltinFunctionCall(override var target: IdentifierReference,
}
override fun copy() = BuiltinFunctionCall(target.copy(), args.map { it.copy() }.toMutableList(), position)
override val isSimple = false
override val isSimple = when (name) {
in arrayOf("msb", "lsb", "mkword", "set_carry", "set_irqd", "clear_carry", "clear_irqd") -> this.args.all { it.isSimple }
else -> false
}
override fun replaceChildNode(node: Node, replacement: Node) {
if(node===target)

View File

@ -53,6 +53,7 @@ Libraries:
Optimizations:
- the many pha/pla's in AssignmentAsmGen added to the code size. Can they be tweaked better? Maybe they are not always required, how to detect that/use another register/tempvar/etc?
- VariableAllocator: can we think of a smarter strategy for allocating variables into zeropage, rather than first-come-first-served?
for instance, vars used inside loops first, then loopvars, then uwords used as pointers, then the rest
- various optimizers skip stuff if compTarget.name==VMTarget.NAME. Once 6502-codegen is done from IR code,
@ -75,6 +76,7 @@ What if we were to re-introduce Structs in prog8? Some thoughts:
Other language/syntax features to think about
---------------------------------------------
- support for assigning multiple return values from romsub/asmsub to multiple variables.
- add (rom/ram)bank support to romsub. A call will then automatically switch banks, use callfar and something else when in banked ram.
challenges: how to not make this too X16 specific? How does the compiler know what bank to switch (ram/rom)?
How to make it performant when we want to (i.e. NOT have it use callfar/auto bank switching) ?

View File

@ -4,18 +4,18 @@
main {
sub start() {
bool @shared a1 = true
bool @shared a2 = false
ubyte @shared rnr = $a0
txt.print_ub(rnr>=$33)
txt.print_ub(rnr>=$66)
txt.print_ub(rnr>=$99)
txt.print_ub(rnr>=$cc)
txt.nl()
txt.print_ub(not a1) ; a1 = a1==0 "0"
ubyte wordNr = (rnr >= $33) as ubyte + (rnr >= $66) as ubyte + (rnr >= $99) as ubyte + (rnr >= $CC) as ubyte
txt.print_uw(wordNr)
txt.nl()
txt.print_ub(not not a1) ; a1 = a1 "1"
txt.nl()
txt.print_ub(not not not a1) ; a1 = a1==0 "0"
txt.nl()
txt.print_ub(not a1 or not a2) ; a1 = a1==0 or a2==0 "1"
txt.nl()
txt.print_ub(not a1 and not a2) ; a1 = a1==0 and a2==0 "0"
wordNr = 100 - (rnr >= $33) - (rnr >= $66) - (rnr >= $99) - (rnr >= $CC)
txt.print_uw(wordNr)
txt.nl()
}
}