From bf0604133ce9e73f9a376ae575939d3e5d5168e9 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sun, 4 Dec 2022 16:02:58 +0100 Subject: [PATCH] fix error in IR for inline asm and BSS vars. --- .../compiler/astprocessing/AstChecker.kt | 2 +- .../compiler/astprocessing/AstExtensions.kt | 4 +- .../astprocessing/BeforeAsmAstChanger.kt | 4 +- .../astprocessing/SymbolTableMaker.kt | 6 +- compiler/test/TestSubroutines.kt | 2 +- .../src/prog8/ast/statements/AstStatements.kt | 9 +- docs/source/todo.rst | 9 +- examples/test.p8 | 115 +++++++----------- .../src/prog8/intermediate/IRFileReader.kt | 3 +- .../src/prog8/vm/VmProgramLoader.kt | 46 ++++--- 10 files changed, 90 insertions(+), 110 deletions(-) diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt index 7337b90fb..7559716c5 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt @@ -251,7 +251,7 @@ internal class AstChecker(private val program: Program, } override fun visit(inlineAssembly: InlineAssembly) { - if(inlineAssembly.hasReturnOrRts(compilerOptions.compTarget)) + if(inlineAssembly.hasReturnOrRts()) count++ } } diff --git a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt index 5933b4ad1..7446825a6 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt @@ -168,9 +168,9 @@ internal fun IdentifierReference.isSubroutineParameter(program: Program): Boolea return false } -internal fun Subroutine.hasRtsInAsm(compTarget: ICompilationTarget): Boolean { +internal fun Subroutine.hasRtsInAsm(): Boolean { return statements .asSequence() .filterIsInstance() - .any { it.hasReturnOrRts(compTarget) } + .any { it.hasReturnOrRts() } } \ No newline at end of file diff --git a/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt b/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt index 12f9df98a..a5dc977b6 100644 --- a/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt +++ b/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt @@ -137,7 +137,7 @@ internal class BeforeAsmAstChanger(val program: Program, mods += IAstModification.InsertLast(returnStmt, subroutine) } else { val last = subroutine.statements.last() - if((last !is InlineAssembly || !last.hasReturnOrRts(options.compTarget)) && last !is Return) { + if((last !is InlineAssembly || !last.hasReturnOrRts()) && last !is Return) { val lastStatement = subroutine.statements.reversed().firstOrNull { it !is Subroutine } if(lastStatement !is Return) { val returnStmt = Return(null, subroutine.position) @@ -164,7 +164,7 @@ internal class BeforeAsmAstChanger(val program: Program, } if (!subroutine.inline || !options.optimize) { - if (subroutine.isAsmSubroutine && subroutine.asmAddress==null && !subroutine.hasRtsInAsm(options.compTarget)) { + if (subroutine.isAsmSubroutine && subroutine.asmAddress==null && !subroutine.hasRtsInAsm()) { // make sure the NOT INLINED asm subroutine actually has a rts at the end // (non-asm routines get a Return statement as needed, above) mods += if(options.compTarget.name==VMTarget.NAME) diff --git a/compiler/src/prog8/compiler/astprocessing/SymbolTableMaker.kt b/compiler/src/prog8/compiler/astprocessing/SymbolTableMaker.kt index 9a416d1b0..9f5b1f56f 100644 --- a/compiler/src/prog8/compiler/astprocessing/SymbolTableMaker.kt +++ b/compiler/src/prog8/compiler/astprocessing/SymbolTableMaker.kt @@ -62,6 +62,8 @@ internal class SymbolTableMaker: IAstVisitor { when(decl.type) { VarDeclType.VAR -> { var initialNumeric = (decl.value as? NumericLiteral)?.number + if(initialNumeric==0.0) + initialNumeric=null // variable will go into BSS and this will be set to 0 val initialStringLit = decl.value as? StringLiteral val initialString = if(initialStringLit==null) null else Pair(initialStringLit.value, initialStringLit.encoding) val initialArrayLit = decl.value as? ArrayLiteral @@ -79,10 +81,8 @@ internal class SymbolTableMaker: IAstVisitor { false else if(decl.isArray) initialArray.isNullOrEmpty() - else { - if(dontReinitGlobals) initialNumeric = initialNumeric ?: 0.0 + else initialNumeric == null - } StStaticVariable(decl.name, decl.datatype, bss, initialNumeric, initialString, initialArray, numElements, decl.zeropage, decl.position) } VarDeclType.CONST -> StConstant(decl.name, decl.datatype, (decl.value as NumericLiteral).number, decl.position) diff --git a/compiler/test/TestSubroutines.kt b/compiler/test/TestSubroutines.kt index cf4cc3c30..446f7fcf6 100644 --- a/compiler/test/TestSubroutines.kt +++ b/compiler/test/TestSubroutines.kt @@ -124,7 +124,7 @@ class TestSubroutines: FunSpec({ asmfunc.isAsmSubroutine shouldBe true asmfunc.statements.single() shouldBe instanceOf() (asmfunc.statements.single() as InlineAssembly).assembly.trim() shouldBe "rts" - asmfunc.hasRtsInAsm(C64Target()) shouldBe true + asmfunc.hasRtsInAsm() shouldBe true func.isAsmSubroutine shouldBe false withClue("str param should have been changed to uword") { asmfunc.parameters.single().type shouldBe DataType.UWORD diff --git a/compilerAst/src/prog8/ast/statements/AstStatements.kt b/compilerAst/src/prog8/ast/statements/AstStatements.kt index 90dce65a5..b268c04ac 100644 --- a/compilerAst/src/prog8/ast/statements/AstStatements.kt +++ b/compilerAst/src/prog8/ast/statements/AstStatements.kt @@ -7,7 +7,6 @@ import prog8.ast.expressions.* import prog8.ast.walk.AstWalker import prog8.ast.walk.IAstVisitor import prog8.code.core.* -import prog8.code.target.VMTarget interface INamedStatement { @@ -637,12 +636,12 @@ class InlineAssembly(val assembly: String, val isIR: Boolean, override val posit override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) - fun hasReturnOrRts(target: ICompilationTarget): Boolean { - return if(target.name!= VMTarget.NAME) { + fun hasReturnOrRts(): Boolean { + return if(isIR) { + " return" in assembly || "\treturn" in assembly || " jump" in assembly || "\tjump" in assembly || " jumpa" in assembly || "\tjumpa" in assembly + } else { " rti" in assembly || "\trti" in assembly || " rts" in assembly || "\trts" in assembly || " jmp" in assembly || "\tjmp" in assembly || " bra" in assembly || "\tbra" in assembly - } else { - " return" in assembly || "\treturn" in assembly || " jump" in assembly || "\tjump" in assembly || " jumpa" in assembly || "\tjumpa" in assembly } } diff --git a/docs/source/todo.rst b/docs/source/todo.rst index b8910b4e2..0d0fdfe4f 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,11 +3,6 @@ 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 - regression test the various projects before release ... @@ -24,6 +19,10 @@ Future Things and Ideas ^^^^^^^^^^^^^^^^^^^^^^^ Compiler: +- 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 +- bss: subroutine parameters don't have to be set to 0. - ir: mechanism to determine for chunks which registers are getting input values from "outside" - ir: mechanism to determine for chunks which registers are passing values out? (i.e. are used again in another chunk) - ir: peephole opt: renumber registers in chunks to start with 1 again every time (but keep entry values in mind!) diff --git a/examples/test.p8 b/examples/test.p8 index 2feaab912..b1cadbd5f 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,79 +1,52 @@ %import textio -%import floats +%zeropage basicsafe +%option no_sysinit main { - sub score() -> ubyte { - cx16.r15++ - return 5 - } + ubyte @requirezp zpvar = 10 + ubyte @zp zpvar2 = 20 + uword empty + ubyte[10] bssarray + uword[10] bsswordarray + ubyte[10] nonbssarray = 99 + str name="irmen" sub start() { - float @shared total = 0 - ubyte bb = 5 + txt.print("= 10 ") + txt.print_ub(zpvar) + txt.nl() + zpvar++ - 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 + 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") } } - - -;%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") -; } -;} diff --git a/intermediate/src/prog8/intermediate/IRFileReader.kt b/intermediate/src/prog8/intermediate/IRFileReader.kt index c2de307b5..fb0754daa 100644 --- a/intermediate/src/prog8/intermediate/IRFileReader.kt +++ b/intermediate/src/prog8/intermediate/IRFileReader.kt @@ -435,7 +435,8 @@ class IRFileReader { val clobbers = attrs.getValue("CLOBBERS") val clobberRegs = if(clobbers.isBlank()) emptyList() else clobbers.split(',').map { CpuRegister.valueOf(it) } - val returns = attrs.getValue("RETURNS").split(',').map { rs -> + val returnsSpec = attrs.getValue("RETURNS") + val returns = if(returnsSpec.isNullOrBlank()) emptyList() else returnsSpec.split(',').map { rs -> val (regstr, dtstr) = rs.split(':') val dt = parseDatatype(dtstr, false) val regsf = parseRegisterOrStatusflag(regstr) diff --git a/virtualmachine/src/prog8/vm/VmProgramLoader.kt b/virtualmachine/src/prog8/vm/VmProgramLoader.kt index 5bbbc0f80..a048026de 100644 --- a/virtualmachine/src/prog8/vm/VmProgramLoader.kt +++ b/virtualmachine/src/prog8/vm/VmProgramLoader.kt @@ -1,8 +1,6 @@ package prog8.vm -import prog8.code.core.ArrayDatatypes -import prog8.code.core.AssemblyError -import prog8.code.core.DataType +import prog8.code.core.* import prog8.intermediate.* class VmProgramLoader { @@ -185,24 +183,34 @@ class VmProgramLoader { program.st.allVariables().forEach { variable -> var addr = allocations.allocations.getValue(variable.name) - if(variable.bss && variable.dt in ArrayDatatypes) { - // zero out the array bss variable. - // non-array variables are reset using explicit assignment instructions. - repeat(variable.length!!) { - when(variable.dt) { - DataType.STR, DataType.ARRAY_UB, DataType.ARRAY_B, DataType.ARRAY_BOOL -> { - memory.setUB(addr, 0u) - addr++ + // zero out BSS variables. + if(variable.bss) { + if(variable.dt in ArrayDatatypes) { + repeat(variable.length!!) { + when(variable.dt) { + DataType.STR, DataType.ARRAY_UB, DataType.ARRAY_B, DataType.ARRAY_BOOL -> { + memory.setUB(addr, 0u) + addr++ + } + DataType.ARRAY_UW, DataType.ARRAY_W -> { + memory.setUW(addr, 0u) + addr += 2 + } + DataType.ARRAY_F -> { + memory.setFloat(addr, 0.0f) + addr += program.options.compTarget.machine.FLOAT_MEM_SIZE + } + else -> throw IRParseException("invalid dt") } - DataType.ARRAY_UW, DataType.ARRAY_W -> { - memory.setUW(addr, 0u) - addr += 2 + } + } else { + if(program.options.dontReinitGlobals) { + when(variable.dt) { + in ByteDatatypes -> memory.setUB(addr, 0u) + in WordDatatypes -> memory.setUW(addr, 0u) + DataType.FLOAT -> memory.setFloat(addr, 0.0f) + else -> throw IRParseException("invalid dt") } - DataType.ARRAY_F -> { - memory.setFloat(addr, 0.0f) - addr += program.options.compTarget.machine.FLOAT_MEM_SIZE - } - else -> throw IRParseException("invalid dt") } } }