From 094f7803b7bcd9e0e015ca6fc0d9ef1c633c8e22 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Thu, 27 Oct 2022 23:08:40 +0200 Subject: [PATCH] fix: array[x] = -array[x] no longer crashes the codegen --- .../src/prog8/codegen/cpu6502/AsmGen.kt | 2 +- .../cpu6502/assignment/AssignmentAsmGen.kt | 31 ++++---- .../assignment/AugmentableAssignmentAsmGen.kt | 25 ++++--- .../astprocessing/AstOnetimeTransforms.kt | 25 ------- .../codegeneration/TestArrayInplaceAssign.kt | 72 +++++++++++++++---- docs/source/todo.rst | 2 +- 6 files changed, 96 insertions(+), 61 deletions(-) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt index de2498560..c89cae6b7 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt @@ -106,7 +106,7 @@ class AsmGen(internal val program: Program, DataType.BYTE -> listOf("cx16", "r9sL") DataType.UWORD -> listOf("cx16", "r9") DataType.WORD -> listOf("cx16", "r9s") - DataType.FLOAT -> listOf("floats", "tempvar_swap_float") // defined in floats.p8 + DataType.FLOAT -> TODO("no temporary float var available") else -> throw FatalAstException("invalid dt $dt") } } diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt index 2df457d1e..68e3e0d4e 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt @@ -300,23 +300,12 @@ internal class AssignmentAsmGen(private val program: Program, val target = virtualRegsToVariables(assign.target) when (value.operator) { "+" -> {} - "-" -> augmentableAsmGen.inplaceNegate(target, target.datatype) + "-" -> augmentableAsmGen.inplaceNegate(assign) "~" -> augmentableAsmGen.inplaceInvert(target, target.datatype) else -> throw AssemblyError("invalid prefix operator") } } else { - // array[x] = -array[x] ... use a tempvar then store that back into the array. - val tempvar = asmgen.getTempVarName(assign.target.datatype).joinToString(".") - val assignToTempvar = AsmAssignment(assign.source, - AsmAssignTarget(TargetStorageKind.VARIABLE, asmgen, assign.target.datatype, assign.target.scope, variableAsmName=tempvar, origAstTarget = assign.target.origAstTarget), - false, program.memsizer, assign.position) - asmgen.translateNormalAssignment(assignToTempvar) - when(assign.target.datatype) { - in ByteDatatypes -> assignVariableByte(assign.target, tempvar) - in WordDatatypes -> assignVariableWord(assign.target, tempvar) - DataType.FLOAT -> assignVariableFloat(assign.target, tempvar) - else -> throw AssemblyError("weird dt") - } + assignPrefixedExpressionToArrayElt(assign) } } is ContainmentCheck -> { @@ -335,6 +324,22 @@ internal class AssignmentAsmGen(private val program: Program, } } + internal fun assignPrefixedExpressionToArrayElt(assign: AsmAssignment) { + // array[x] = -value ... use a tempvar then store that back into the array. + require(assign.source.expression is PrefixExpression) + val tempvar = asmgen.getTempVarName(assign.target.datatype).joinToString(".") + val assignToTempvar = AsmAssignment(assign.source, + AsmAssignTarget(TargetStorageKind.VARIABLE, asmgen, assign.target.datatype, assign.target.scope, variableAsmName=tempvar, origAstTarget = assign.target.origAstTarget), + false, program.memsizer, assign.position) + asmgen.translateNormalAssignment(assignToTempvar) + when(assign.target.datatype) { + in ByteDatatypes -> assignVariableByte(assign.target, tempvar) + in WordDatatypes -> assignVariableWord(assign.target, tempvar) + DataType.FLOAT -> assignVariableFloat(assign.target, tempvar) + else -> throw AssemblyError("weird dt") + } + } + private fun assignVirtualRegister(target: AsmAssignTarget, register: RegisterOrPair) { when(target.datatype) { in ByteDatatypes -> { diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt index 5eade9bd1..c21c51e7b 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt @@ -26,7 +26,7 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, val type = itype.getOrElse { throw AssemblyError("unknown dt") } when (value.operator) { "+" -> {} - "-" -> inplaceNegate(target, type) + "-" -> inplaceNegate(assign) "~" -> inplaceInvert(target, type) else -> throw AssemblyError("invalid prefix operator") } @@ -1840,7 +1840,8 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, } } TargetStorageKind.STACK -> TODO("no asm gen for byte stack invert") - else -> TODO("no asm gen for in-place invert ubyte for ${target.kind}") + TargetStorageKind.ARRAY -> TODO("no asm gen for in-place invert ubyte for ${target.kind}") + else -> throw AssemblyError("weird target") } } DataType.UWORD -> { @@ -1864,15 +1865,17 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, } } TargetStorageKind.STACK -> TODO("no asm gen for word stack invert") - else -> TODO("no asm gen for in-place invert uword for ${target.kind}") + TargetStorageKind.ARRAY -> TODO("no asm gen for in-place invert uword for ${target.kind}") + else -> throw AssemblyError("weird target") } } else -> throw AssemblyError("invert of invalid type") } } - internal fun inplaceNegate(target: AsmAssignTarget, dt: DataType) { - when (dt) { + internal fun inplaceNegate(assign: AsmAssignment) { + val target = assign.target + when (val dt=assign.target.datatype) { DataType.BYTE -> { when (target.kind) { TargetStorageKind.VARIABLE -> { @@ -1896,9 +1899,10 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, else -> throw AssemblyError("invalid reg dt for byte negate") } } - TargetStorageKind.MEMORY -> throw AssemblyError("memory is ubyte, can't in-place negate") + TargetStorageKind.MEMORY -> throw AssemblyError("memory is ubyte, can't negate that") TargetStorageKind.STACK -> TODO("no asm gen for byte stack negate") - else -> TODO("no asm gen for in-place negate byte") + TargetStorageKind.ARRAY -> assignmentAsmGen.assignPrefixedExpressionToArrayElt(assign) + else -> throw AssemblyError("weird target") } } DataType.WORD -> { @@ -1955,8 +1959,10 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, else -> throw AssemblyError("invalid reg dt for word neg") } } + TargetStorageKind.MEMORY -> throw AssemblyError("memory is ubyte, can't negate that") TargetStorageKind.STACK -> TODO("no asm gen for word stack negate") - else -> TODO("no asm gen for in-place negate word") + TargetStorageKind.ARRAY -> assignmentAsmGen.assignPrefixedExpressionToArrayElt(assign) + else -> throw AssemblyError("weird target") } } DataType.FLOAT -> { @@ -1970,7 +1976,8 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, """) } TargetStorageKind.STACK -> TODO("no asm gen for float stack negate") - else -> TODO("no asmgen for inplace negate float ${target.kind}") + TargetStorageKind.ARRAY -> assignmentAsmGen.assignPrefixedExpressionToArrayElt(assign) + else -> throw AssemblyError("weird target for float negation") } } else -> throw AssemblyError("negate of invalid type $dt") diff --git a/compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt b/compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt index 69dd0f47d..75fb8256b 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt @@ -16,31 +16,6 @@ import prog8.code.target.VMTarget internal class AstOnetimeTransforms(private val program: Program, private val options: CompilationOptions) : AstWalker() { - override fun after(assignment: Assignment, parent: Node): Iterable { - if(options.compTarget.name != VMTarget.NAME) { - // The 6502 code-gen doesn't contain code to deal with arr[0] = -arr[0] and arr[0] = ~arr[0]. - // Replace this by assignment, operation, assignment. - // TODO make the codegen better so this work around can be removed - -// if (assignment.isAugmentable) { -// val arrayIdx = assignment.target.arrayindexed -// if (arrayIdx != null) { -// val prefixed = assignment.value as? PrefixExpression -// if (prefixed != null) { -// println("array self-assignment, operator = ${prefixed.operator}") -// if (prefixed.operator == "-") { -// TODO("array in-place -") -// } else if (prefixed.operator == "~") { -// TODO("array in-place ~") -// } -// } -// } -// } - - } - return noModifications - } - override fun after(arrayIndexedExpression: ArrayIndexedExpression, parent: Node): Iterable { if(parent !is VarDecl) { if(options.compTarget.name == VMTarget.NAME) diff --git a/compiler/test/codegeneration/TestArrayInplaceAssign.kt b/compiler/test/codegeneration/TestArrayInplaceAssign.kt index 75910601f..f551a5a98 100644 --- a/compiler/test/codegeneration/TestArrayInplaceAssign.kt +++ b/compiler/test/codegeneration/TestArrayInplaceAssign.kt @@ -1,20 +1,9 @@ package prog8tests.codegeneration import io.kotest.core.spec.style.FunSpec -import io.kotest.core.spec.style.StringSpec import io.kotest.matchers.shouldNotBe -import prog8.ast.Module -import prog8.ast.Program -import prog8.ast.expressions.AddressOf -import prog8.ast.expressions.IdentifierReference -import prog8.ast.expressions.NumericLiteral -import prog8.ast.statements.* -import prog8.code.core.* import prog8.code.target.C64Target -import prog8.code.target.c64.C64Zeropage -import prog8.codegen.cpu6502.AsmGen -import prog8.compiler.astprocessing.SymbolTableMaker -import prog8tests.helpers.* +import prog8tests.helpers.compileText class TestArrayInplaceAssign: FunSpec({ test("assign prefix var to array should compile fine and is not split into inplace array modification") { @@ -29,5 +18,64 @@ class TestArrayInplaceAssign: FunSpec({ """ compileText(C64Target(), false, text, writeAssembly = true) shouldNotBe null } + + test("array in-place negation (integer types)") { + val text = """ +%import floats + +main { + byte[10] foo + ubyte[10] foou + word[10] foow + uword[10] foowu + float[10] flt + + sub start() { + foo[1] = 42 + foo[1] = -foo[1] + + foow[1] = 4242 + foow[1] = -foow[1] + + ; TODO floating point in-place negation is not yet implemented in the code generator + ; flt[1] = 42.42 + ; flt[1] = -flt[1] + } +}""" + compileText(C64Target(), false, text, writeAssembly = true) shouldNotBe null + } + + // TODO implement this in codegen and enable test + xtest("array in-place negation (float type) - ignored for now because not implemented in codegen yet") { + val text = """ +%import floats + +main { + float[10] flt + + sub start() { + flt[1] = 42.42 + flt[1] = -flt[1] + } +}""" + compileText(C64Target(), false, text, writeAssembly = true) shouldNotBe null + } + + test("array in-place invert") { + val text = """ +main { + ubyte[10] foo + uword[10] foow + + sub start() { + foo[1] = 42 + foo[1] = ~foo[1] + + foow[1] = 4242 + foow[1] = ~foow[1] + } +}""" + compileText(C64Target(), false, text, writeAssembly = true) shouldNotBe null + } }) diff --git a/docs/source/todo.rst b/docs/source/todo.rst index f062b5b23..6c85d5ec6 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,7 +3,7 @@ TODO For next release ^^^^^^^^^^^^^^^^ -- fix the array in place assignment issue, see AstOnetimeTransforms +- fix the array in place assignment issue - ir: asmsub contents remains blank in IR file