From 893b383bdfe5b50c657d0017249ac428ee32e6fe Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Tue, 17 Oct 2023 02:34:56 +0200 Subject: [PATCH] fix signed byte to word sign extension in assignment --- .../cpu6502/assignment/AssignmentAsmGen.kt | 74 ++++++++++--------- .../astprocessing/BeforeAsmAstChanger.kt | 68 ----------------- compiler/test/TestTypecasts.kt | 7 +- .../test/codegeneration/TestVariousCodeGen.kt | 3 +- docs/source/todo.rst | 3 +- examples/test.p8 | 14 +++- 6 files changed, 58 insertions(+), 111 deletions(-) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt index f3866622f..aedf7b9c0 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt @@ -913,10 +913,34 @@ internal class AssignmentAsmGen(private val program: PtProgram, is PtTypeCast -> { val castedValue = right.value if(right.type in WordDatatypes && castedValue.type in ByteDatatypes && castedValue is PtIdentifier) { - val castedSymname = asmgen.asmVariableName(castedValue) - assignExpressionToRegister(left, RegisterOrPair.AY, dt == DataType.WORD) - if (expr.operator == "+") - asmgen.out( + if(right.type in SignedDatatypes) { + // we need to sign extend, do this via temporary word variable + asmgen.assignExpressionToVariable(right, "P8ZP_SCRATCH_W1", DataType.WORD) + assignExpressionToRegister(left, RegisterOrPair.AY, dt == DataType.WORD) + if(expr.operator=="+") { + asmgen.out(""" + clc + adc P8ZP_SCRATCH_W1 + tax + tya + adc P8ZP_SCRATCH_W1+1 + tay + txa""") + } else if(expr.operator=="-") { + asmgen.out(""" + sec + sbc P8ZP_SCRATCH_W1 + tax + tya + sbc P8ZP_SCRATCH_W1+1 + tay + txa""") + } + } else { + assignExpressionToRegister(left, RegisterOrPair.AY, dt == DataType.WORD) + val castedSymname = asmgen.asmVariableName(castedValue) + if (expr.operator == "+") + asmgen.out( """ clc adc $castedSymname @@ -924,8 +948,8 @@ internal class AssignmentAsmGen(private val program: PtProgram, iny +""" ) - else - asmgen.out( + else + asmgen.out( """ sec sbc $castedSymname @@ -933,6 +957,7 @@ internal class AssignmentAsmGen(private val program: PtProgram, dey +""" ) + } assignRegisterpairWord(target, RegisterOrPair.AY) return true } @@ -1844,13 +1869,13 @@ internal class AssignmentAsmGen(private val program: PtProgram, assignCastViaLsbFunc(value, target) } else if(valueDt in WordDatatypes && targetDt in WordDatatypes) { // word to word, just assign - assignExpressionToRegister(value, target.register!!, targetDt==DataType.BYTE || targetDt==DataType.WORD) + assignExpressionToRegister(value, target.register!!, valueDt in SignedDatatypes) } else if(valueDt in ByteDatatypes && targetDt in ByteDatatypes) { // byte to byte, just assign - assignExpressionToRegister(value, target.register!!, targetDt==DataType.BYTE || targetDt==DataType.WORD) + assignExpressionToRegister(value, target.register!!, valueDt in SignedDatatypes) } else if(valueDt in ByteDatatypes && targetDt in WordDatatypes) { // byte to word, just assign - assignExpressionToRegister(value, target.register!!, targetDt==DataType.WORD) + assignExpressionToRegister(value, target.register!!, valueDt==DataType.WORD) } else throw AssemblyError("can't cast $valueDt to $targetDt, this should have been checked in the astchecker") } @@ -2299,6 +2324,13 @@ internal class AssignmentAsmGen(private val program: PtProgram, } private fun assignVariableWord(target: AsmAssignTarget, sourceName: String, sourceDt: DataType) { + if(sourceDt==DataType.BYTE) { + // need to sign extend + asmgen.out(" lda $sourceName") + asmgen.signExtendAYlsb(DataType.BYTE) + assignRegisterpairWord(target, RegisterOrPair.AY) + return + } require(sourceDt in WordDatatypes || sourceDt==DataType.UBYTE) when(target.kind) { TargetStorageKind.VARIABLE -> { @@ -2342,16 +2374,6 @@ internal class AssignmentAsmGen(private val program: PtProgram, lda $sourceName+1 sta ${target.asmVarname}+$scaledIdx+1""") } - DataType.FLOAT -> { - asmgen.out(""" - lda #<$sourceName - ldy #>$sourceName - sta P8ZP_SCRATCH_W1 - sty P8ZP_SCRATCH_W1+1 - lda #<(${target.asmVarname}+$scaledIdx) - ldy #>(${target.asmVarname}+$scaledIdx) - jsr floats.copy_float""") - } else -> throw AssemblyError("weird target variable type ${target.datatype}") } } @@ -2377,20 +2399,6 @@ internal class AssignmentAsmGen(private val program: PtProgram, lda $sourceName+1 sta ${target.asmVarname}+1,y""") } - DataType.FLOAT -> { - asmgen.loadScaledArrayIndexIntoRegister(target.array, target.datatype, CpuRegister.A) - asmgen.out(""" - ldy #<$sourceName - sty P8ZP_SCRATCH_W1 - ldy #>$sourceName - sty P8ZP_SCRATCH_W1+1 - ldy #>${target.asmVarname} - clc - adc #<${target.asmVarname} - bcc + - iny -+ jsr floats.copy_float""") - } else -> throw AssemblyError("weird dt") } } diff --git a/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt b/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt index 1c140223d..e568c6d8e 100644 --- a/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt +++ b/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt @@ -39,74 +39,6 @@ internal class BeforeAsmAstChanger(val program: Program, private val options: Co return noModifications } -/* TODO remove permanently: - override fun after(assignment: Assignment, parent: Node): Iterable { - // Try to replace A = B Something by A= B, A = A Something - // this triggers the more efficent augmented assignment code generation more often. - // But it can only be done if the target variable IS NOT OCCURRING AS AN OPERAND ITSELF. - - if(options.compTarget.name==VMTarget.NAME) // don't apply this optimization for Vm target - return noModifications - - if(!assignment.isAugmentable - && assignment.target.identifier != null - && !assignment.target.isIOAddress(options.compTarget.machine)) { - val binExpr = assignment.value as? BinaryExpression - - if (binExpr != null && binExpr.operator !in ComparisonOperators) { - if (binExpr.left !is BinaryExpression) { - if (binExpr.right.referencesIdentifier(assignment.target.identifier!!.nameInSource)) { - // the right part of the expression contains the target variable itself. - // we can't 'split' it trivially because the variable will be changed halfway through. - if(binExpr.operator in AssociativeOperators) { - // A = - // use the other part of the expression to split. - val sourceDt = binExpr.right.inferType(program).getOrElse { throw AssemblyError("unknown dt") } - val (_, right) = binExpr.right.typecastTo(assignment.target.inferType(program).getOrElse { throw AssemblyError( - "unknown dt" - ) - }, sourceDt, implicit=true) - val assignRight = Assignment(assignment.target, right, AssignmentOrigin.ASMGEN, assignment.position) - return listOf( - IAstModification.InsertBefore(assignment, assignRight, parent as IStatementContainer), - IAstModification.ReplaceNode(binExpr.right, binExpr.left, binExpr), - IAstModification.ReplaceNode(binExpr.left, assignment.target.toExpression(), binExpr) - ) - } - } else { - if(binExpr.left isSameAs assignment.target) - return noModifications - val typeCast = binExpr.left as? TypecastExpression - if(typeCast!=null && typeCast.expression isSameAs assignment.target) - return noModifications - - if(binExpr.operator in "+-") { - val leftDt = binExpr.left.inferType(program) - val rightDt = binExpr.right.inferType(program) - if(leftDt==rightDt && leftDt.isInteger && rightDt.isInteger && binExpr.right is ArrayIndexedExpression) { - // don't split array[i] +/- array[i] (the codegen has an optimized path for this) - return noModifications - } - } - - val sourceDt = binExpr.left.inferType(program).getOrElse { throw AssemblyError("unknown dt") } - val (_, left) = binExpr.left.typecastTo(assignment.target.inferType(program).getOrElse { throw AssemblyError( - "unknown dt" - ) - }, sourceDt, implicit=true) - val assignLeft = Assignment(assignment.target, left, AssignmentOrigin.ASMGEN, assignment.position) - return listOf( - IAstModification.InsertBefore(assignment, assignLeft, parent as IStatementContainer), - IAstModification.ReplaceNode(binExpr.left, assignment.target.toExpression(), binExpr) - ) - } - } - } - } - return noModifications - } -*/ - override fun after(scope: AnonymousScope, parent: Node): Iterable { if(scope.statements.any { it is VarDecl || it is IStatementContainer }) throw FatalAstException("anonymousscope may no longer contain any vardecls or subscopes") diff --git a/compiler/test/TestTypecasts.kt b/compiler/test/TestTypecasts.kt index 52464c000..348916537 100644 --- a/compiler/test/TestTypecasts.kt +++ b/compiler/test/TestTypecasts.kt @@ -58,11 +58,10 @@ class TestTypecasts: FunSpec({ val result2 = compileText(C64Target(), true, text, writeAssembly = true)!! val stmts2 = result2.compilerAst.entrypoint.statements - stmts2.size shouldBe 6 - val expr2 = (stmts2[4] as Assignment).value as BinaryExpression + stmts2.size shouldBe 5 + val expr2 = (stmts2[3] as Assignment).value as BinaryExpression expr2.operator shouldBe "&" expr2.right shouldBe NumericLiteral(DataType.UBYTE, 1.0, Position.DUMMY) - (expr2.left as IdentifierReference).nameInSource shouldBe listOf("bb") } test("bool expressions with functioncalls") { @@ -740,7 +739,7 @@ main { } """ val result = compileText(C64Target(), false, text, writeAssembly = true)!! - result.compilerAst.entrypoint.statements.size shouldBe 15 + result.compilerAst.entrypoint.statements.size shouldBe 14 } test("invalid typecasts of numbers") { diff --git a/compiler/test/codegeneration/TestVariousCodeGen.kt b/compiler/test/codegeneration/TestVariousCodeGen.kt index 54de41379..373d8c011 100644 --- a/compiler/test/codegeneration/TestVariousCodeGen.kt +++ b/compiler/test/codegeneration/TestVariousCodeGen.kt @@ -9,6 +9,7 @@ import io.kotest.matchers.string.shouldStartWith import io.kotest.matchers.types.instanceOf import prog8.code.ast.PtArrayIndexer import prog8.code.ast.PtAssignment +import prog8.code.ast.PtBinaryExpression import prog8.code.ast.PtVariable import prog8.code.core.DataType import prog8.code.target.C64Target @@ -93,7 +94,7 @@ main { seed.type shouldBe DataType.ARRAY_UW val assign = start.children[1] as PtAssignment assign.target.identifier!!.name shouldBe "cx16.r0" - assign.value shouldBe instanceOf() + assign.value shouldBe instanceOf() } test("peek and poke argument types") { diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 9fce3508e..64d705980 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,8 +1,7 @@ TODO ==== -- monogfx/gfx2 flood fill is broken after removing the after(assignment: Assignment) from BeforeAsmAstChanger that splits assignments - also other things broken? rectangle? not sure +- fix codegen signed byte to word casting issue uw = 8888 + (bb as ubyte) - remove after(assignment from BeforeAsmAstChanger permanently once issues above fixed - gfx2/monogfx: use vera auto in/decrement in the flood fill routine (getpixels) diff --git a/examples/test.p8 b/examples/test.p8 index 419872e45..788fabd4f 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,10 +1,18 @@ +%import textio %option no_sysinit %zeropage basicsafe main { - sub start() { - uword xx - cx16.r1L = lsb(xx) & 3 + uword @shared uw = 5555 + byte @shared bb = -44 + + uw = (bb as ubyte) as uword + txt.print_uw(uw) ; 212 + txt.nl() + + uw = 8888 + (bb as ubyte) ; TODO fix 6502 codegen + txt.print_uw(uw) ; 9100 + txt.nl() } }