From a82b2da16e1e6e0c76180c3fbf387630f4316f6f Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sun, 4 Dec 2022 12:22:05 +0100 Subject: [PATCH] Fix some FP related assignment issues in 6502 codegen. --- .../cpu6502/assignment/AssignmentAsmGen.kt | 75 +++++++---- compiler/src/prog8/CompilerMain.kt | 2 +- compiler/test/TestTypecasts.kt | 31 +++++ docs/source/building.rst | 4 +- docs/source/todo.rst | 2 + examples/test.p8 | 117 +++++++++++------- 6 files changed, 160 insertions(+), 71 deletions(-) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt index 6fb15f63a..8e937b559 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt @@ -27,13 +27,6 @@ internal class AssignmentAsmGen(private val program: Program, translateNormalAssignment(assign) } - internal fun virtualRegsToVariables(origtarget: AsmAssignTarget): AsmAssignTarget { - return if(origtarget.kind==TargetStorageKind.REGISTER && origtarget.register in Cx16VirtualRegisters) { - AsmAssignTarget(TargetStorageKind.VARIABLE, asmgen, origtarget.datatype, origtarget.scope, - variableAsmName = "cx16.${origtarget.register!!.name.lowercase()}", origAstTarget = origtarget.origAstTarget) - } else origtarget - } - fun translateNormalAssignment(assign: AsmAssignment) { if(assign.isAugmentable) { augmentableAsmGen.translate(assign) @@ -940,11 +933,9 @@ internal class AssignmentAsmGen(private val program: Program, is NumericLiteral -> { val address = (value.addressExpression as NumericLiteral).number.toUInt() assignMemoryByteIntoWord(target, address, null) - return } is IdentifierReference -> { assignMemoryByteIntoWord(target, null, value.addressExpression as IdentifierReference) - return } is BinaryExpression -> { if(asmgen.tryOptimizedPointerAccessWithA(value.addressExpression as BinaryExpression, false)) { @@ -958,6 +949,7 @@ internal class AssignmentAsmGen(private val program: Program, assignViaExprEval(value.addressExpression) } } + return } } is NumericLiteral -> throw AssemblyError("a cast of a literal value should have been const-folded away") @@ -966,7 +958,7 @@ internal class AssignmentAsmGen(private val program: Program, // special case optimizations - if(target.kind== TargetStorageKind.VARIABLE) { + if(target.kind == TargetStorageKind.VARIABLE) { if(value is IdentifierReference && valueDt != DataType.UNDEFINED) return assignTypeCastedIdentifier(target.asmVarname, targetDt, asmgen.asmVariableName(value), valueDt) @@ -1044,11 +1036,12 @@ internal class AssignmentAsmGen(private val program: Program, assignExpressionToRegister(value, RegisterOrPair.FAC1, target.datatype in SignedDatatypes) assignTypeCastedFloatFAC1("P8ZP_SCRATCH_W1", target.datatype) assignVariableToRegister("P8ZP_SCRATCH_W1", target.register!!, target.datatype in SignedDatatypes) + return } else { if(!(valueDt isAssignableTo targetDt)) { - if(valueDt in WordDatatypes && targetDt in ByteDatatypes) { + return if(valueDt in WordDatatypes && targetDt in ByteDatatypes) { // word to byte, just take the lsb - return assignCastViaLsbFunc(value, target) + 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) @@ -1058,43 +1051,85 @@ internal class AssignmentAsmGen(private val program: Program, } else if(valueDt in ByteDatatypes && targetDt in WordDatatypes) { // byte to word, just assign assignExpressionToRegister(value, target.register!!, targetDt==DataType.WORD) - } - else + } else throw AssemblyError("can't cast $valueDt to $targetDt, this should have been checked in the astchecker") } - assignExpressionToRegister(value, target.register!!, targetDt==DataType.BYTE || targetDt==DataType.WORD) } - return + } + + if(targetDt in IntegerDatatypes && valueDt in IntegerDatatypes && valueDt.isAssignableTo(targetDt)) { + require(targetDt in WordDatatypes && valueDt in ByteDatatypes) { "should be byte to word assignment ${origTypeCastExpression.position}"} + when(target.kind) { +// TargetStorageKind.VARIABLE -> { +// This has been handled already earlier on line 961. +// // byte to word, just assign to registers first, then assign to variable +// assignExpressionToRegister(value, RegisterOrPair.AY, targetDt==DataType.WORD) +// assignTypeCastedRegisters(target.asmVarname, targetDt, RegisterOrPair.AY, targetDt) +// return +// } + TargetStorageKind.ARRAY -> { + // byte to word, just assign to registers first, then assign into array + assignExpressionToRegister(value, RegisterOrPair.AY, targetDt==DataType.WORD) + assignRegisterpairWord(target, RegisterOrPair.AY) + return + } + TargetStorageKind.REGISTER -> { + // byte to word, just assign to registers + assignExpressionToRegister(value, target.register!!, targetDt==DataType.WORD) + return + } + TargetStorageKind.STACK -> { + // byte to word, just assign to registers first, then push onto stack + assignExpressionToRegister(value, RegisterOrPair.AY, targetDt==DataType.WORD) + asmgen.out(""" + sta P8ESTACK_LO,x + tya + sta P8ESTACK_HI,x + dex""") + return + } + else -> throw AssemblyError("weird target") + } } if(targetDt==DataType.FLOAT && (target.register==RegisterOrPair.FAC1 || target.register==RegisterOrPair.FAC2)) { when(valueDt) { DataType.UBYTE -> { assignExpressionToRegister(value, RegisterOrPair.Y, false) + asmgen.saveRegisterLocal(CpuRegister.X, origTypeCastExpression.definingSubroutine!!) asmgen.out(" jsr floats.FREADUY") + asmgen.restoreRegisterLocal(CpuRegister.X) } DataType.BYTE -> { assignExpressionToRegister(value, RegisterOrPair.A, true) + asmgen.saveRegisterLocal(CpuRegister.X, origTypeCastExpression.definingSubroutine!!) asmgen.out(" jsr floats.FREADSA") + asmgen.restoreRegisterLocal(CpuRegister.X) } DataType.UWORD -> { assignExpressionToRegister(value, RegisterOrPair.AY, false) + asmgen.saveRegisterLocal(CpuRegister.X, origTypeCastExpression.definingSubroutine!!) asmgen.out(" jsr floats.GIVUAYFAY") + asmgen.restoreRegisterLocal(CpuRegister.X) } DataType.WORD -> { assignExpressionToRegister(value, RegisterOrPair.AY, true) + asmgen.saveRegisterLocal(CpuRegister.X, origTypeCastExpression.definingSubroutine!!) asmgen.out(" jsr floats.GIVAYFAY") + asmgen.restoreRegisterLocal(CpuRegister.X) } else -> throw AssemblyError("invalid dt") } if(target.register==RegisterOrPair.FAC2) { asmgen.out(" jsr floats.MOVEF") } + return } else { // No more special optmized cases yet. Do the rest via more complex evaluation // note: cannot use assignTypeCastedValue because that is ourselves :P // NOTE: THIS MAY TURN INTO A STACK OVERFLOW ERROR IF IT CAN'T SIMPLIFY THE TYPECAST..... :-/ asmgen.assignExpressionTo(origTypeCastExpression, target) + return } } @@ -1251,14 +1286,10 @@ internal class AssignmentAsmGen(private val program: Program, DataType.UWORD, DataType.WORD -> { if(asmgen.isTargetCpu(CpuType.CPU65c02)) asmgen.out( - " st${ - regs.toString().lowercase() - } $targetAsmVarName | stz $targetAsmVarName+1") + " st${regs.toString().lowercase()} $targetAsmVarName | stz $targetAsmVarName+1") else asmgen.out( - " st${ - regs.toString().lowercase() - } $targetAsmVarName | lda #0 | sta $targetAsmVarName+1") + " st${regs.toString().lowercase()} $targetAsmVarName | lda #0 | sta $targetAsmVarName+1") } DataType.FLOAT -> { when(regs) { diff --git a/compiler/src/prog8/CompilerMain.kt b/compiler/src/prog8/CompilerMain.kt index 607b5dc31..626847b33 100644 --- a/compiler/src/prog8/CompilerMain.kt +++ b/compiler/src/prog8/CompilerMain.kt @@ -44,7 +44,7 @@ private fun compileMain(args: Array): Boolean { val keepIR by cli.option(ArgType.Boolean, fullName = "keepIR", description = "keep the IR code file (for targets that use it)") val dontWriteAssembly by cli.option(ArgType.Boolean, fullName = "noasm", description="don't create assembly code") val dontOptimize by cli.option(ArgType.Boolean, fullName = "noopt", description = "don't perform any optimizations") - val dontReinitGlobals by cli.option(ArgType.Boolean, fullName = "noreinit", description = "don't create code to reinitialize globals on multiple runs of the program (experimental!)") + val dontReinitGlobals by cli.option(ArgType.Boolean, fullName = "noreinit", description = "don't create code to reinitialize globals on multiple runs of the program (experimental)") val outputDir by cli.option(ArgType.String, fullName = "out", description = "directory for output files instead of current directory").default(".") val optimizeFloatExpressions by cli.option(ArgType.Boolean, fullName = "optfloatx", description = "optimize float expressions (warning: can increase program size)") val quietAssembler by cli.option(ArgType.Boolean, fullName = "quietasm", description = "don't print assembler output results") diff --git a/compiler/test/TestTypecasts.kt b/compiler/test/TestTypecasts.kt index 32d8ccc42..5956eb533 100644 --- a/compiler/test/TestTypecasts.kt +++ b/compiler/test/TestTypecasts.kt @@ -14,6 +14,7 @@ import prog8.ast.statements.VarDecl import prog8.code.core.DataType import prog8.code.core.Position import prog8.code.target.C64Target +import prog8.code.target.VMTarget import prog8tests.helpers.ErrorReporterForTests import prog8tests.helpers.compileText @@ -934,4 +935,34 @@ main { }""" compileText(C64Target(), false, text, writeAssembly = true) shouldNotBe null } + + test("various floating point casts don't crash the compiler") { + val text=""" + %import floats + + main { + sub score() -> ubyte { + cx16.r15++ + return 5 + } + + sub start() { + float @shared total = 0 + ubyte bb = 5 + + cx16.r0 = 5 + total += cx16.r0 as float + total += score() as float + uword ww = 5 + total += ww as float + total += bb as float + float result = score() as float + total += result + } + }""" + compileText(C64Target(), false, text, writeAssembly = true) shouldNotBe null + compileText(C64Target(), true, text, writeAssembly = true) shouldNotBe null + compileText(VMTarget(), false, text, writeAssembly = true) shouldNotBe null + compileText(VMTarget(), true, text, writeAssembly = true) shouldNotBe null + } }) diff --git a/docs/source/building.rst b/docs/source/building.rst index e5aeb1adc..9dc602fb5 100644 --- a/docs/source/building.rst +++ b/docs/source/building.rst @@ -140,8 +140,8 @@ One or more .p8 module files Don't create code to reinitialize the global (block level) variables on every run of the program. Also means that all such variables are no longer placed in the zeropage. Sometimes the program will be a lot shorter when using this, but sometimes the opposite happens. - When using this option, it is no longer be possible to run the program correctly more than once! - *Experimental feature*: still has some problems! + When using this option, it may no longer be possible to run the program correctly more than once! + *Experimental feature*: this feature has not been tested much yet. ``-optfloatx`` Also optimize float expressions if optimizations are enabled. diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 27e4cb391..b8910b4e2 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,6 +3,8 @@ TODO For next release ^^^^^^^^^^^^^^^^ +- fix compiler crash when compiling %import test_stack on virtual target. +- bss in IR: with -noreinit, variables that have init value 0 should still be bss. - 6502 codegen: create BSS section in output assembly code and put StStaticVariables in there with bss=true. Don't forget to add init code to zero out everything that was put in bss. If array in bss->only zero ONCE if possible. Note that bss can still contain variables that have @zp tag and those are already dealt with differently diff --git a/examples/test.p8 b/examples/test.p8 index 34653ead5..2feaab912 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,54 +1,79 @@ %import textio -%zeropage basicsafe -%option no_sysinit +%import floats main { + sub score() -> ubyte { + cx16.r15++ + return 5 + } sub start() { - ; TODO ALSO TEST AS GLOBALS - ubyte @requirezp zpvar = 10 - ubyte @zp zpvar2 = 20 - uword empty - ubyte[10] bssarray - uword[10] bsswordarray - ubyte[10] nonbssarray = 99 - str name="irmen" + float @shared total = 0 + ubyte bb = 5 - txt.print("10 ") - txt.print_ub(zpvar) - txt.nl() - zpvar++ - - txt.print("20 ") - txt.print_ub(zpvar2) - txt.nl() - zpvar2++ - - txt.print("0 ") - txt.print_uw(empty) - txt.nl() - empty++ - - txt.print("0 ") - txt.print_ub(bssarray[1]) - txt.nl() - bssarray[1]++ - - txt.print("0 ") - txt.print_uw(bsswordarray[1]) - txt.nl() - bsswordarray[1]++ - - txt.print("99 ") - txt.print_ub(nonbssarray[1]) - txt.nl() - nonbssarray[1]++ - - txt.print("r ") - txt.chrout(name[1]) - txt.nl() - name[1] = (name[1] as ubyte +1) - - txt.print("try running again.\n") + cx16.r0 = 5 + total += cx16.r0 as float + total += score() as float + uword ww = 5 + total += ww as float + total += bb as float + float result = score() as float + total += result } } + + +;%import textio +;%zeropage basicsafe +;%option no_sysinit +; +;main { +; +; sub start() { +; ; TODO ALSO TEST AS GLOBALS +; ubyte @requirezp zpvar = 10 +; ubyte @zp zpvar2 = 20 +; uword empty +; ubyte[10] bssarray +; uword[10] bsswordarray +; ubyte[10] nonbssarray = 99 +; str name="irmen" +; +; txt.print("10 ") +; txt.print_ub(zpvar) +; txt.nl() +; zpvar++ +; +; txt.print("20 ") +; txt.print_ub(zpvar2) +; txt.nl() +; zpvar2++ +; +; txt.print("0 ") +; txt.print_uw(empty) +; txt.nl() +; empty++ +; +; txt.print("0 ") +; txt.print_ub(bssarray[1]) +; txt.nl() +; bssarray[1]++ +; +; txt.print("0 ") +; txt.print_uw(bsswordarray[1]) +; txt.nl() +; bsswordarray[1]++ +; +; txt.print("99 ") +; txt.print_ub(nonbssarray[1]) +; txt.nl() +; nonbssarray[1]++ +; +; txt.print("r ") +; txt.chrout(name[1]) +; txt.nl() +; name[1] = (name[1] as ubyte +1) +; +; txt.print("try running again.\n") +; } +;}