From b4fa72c0580f4787ba200aa94fe1f126dbeb6514 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Wed, 3 Nov 2021 00:47:22 +0100 Subject: [PATCH] fix parent node linkage for reading array parameter --- .../astprocessing/AstVariousTransforms.kt | 6 +- .../astprocessing/StatementReorderer.kt | 7 +- compiler/test/TestSubroutines.kt | 83 +++++++++++++++++-- docs/source/todo.rst | 11 ++- examples/test.p8 | 62 +------------- 5 files changed, 96 insertions(+), 73 deletions(-) diff --git a/compiler/src/prog8/compiler/astprocessing/AstVariousTransforms.kt b/compiler/src/prog8/compiler/astprocessing/AstVariousTransforms.kt index d2938c02a..1b7d6a57d 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstVariousTransforms.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstVariousTransforms.kt @@ -67,7 +67,7 @@ internal class AstVariousTransforms(private val program: Program) : AstWalker() } override fun after(arrayIndexedExpression: ArrayIndexedExpression, parent: Node): Iterable { - return replacePointerVarIndexWithMemread(program, arrayIndexedExpression, parent) + return replacePointerVarIndexWithMemreadOrMemwrite(program, arrayIndexedExpression, parent) } private fun concatString(expr: BinaryExpression): StringLiteralValue? { @@ -99,12 +99,12 @@ internal class AstVariousTransforms(private val program: Program) : AstWalker() -internal fun replacePointerVarIndexWithMemread(program: Program, arrayIndexedExpression: ArrayIndexedExpression, parent: Node): Iterable { +internal fun replacePointerVarIndexWithMemreadOrMemwrite(program: Program, arrayIndexedExpression: ArrayIndexedExpression, parent: Node): Iterable { val arrayVar = arrayIndexedExpression.arrayvar.targetVarDecl(program) if(arrayVar!=null && arrayVar.datatype == DataType.UWORD) { // rewrite pointervar[index] into @(pointervar+index) val indexer = arrayIndexedExpression.indexer - val add = BinaryExpression(arrayIndexedExpression.arrayvar, "+", indexer.indexExpr, arrayIndexedExpression.position) + val add = BinaryExpression(arrayIndexedExpression.arrayvar.copy(), "+", indexer.indexExpr, arrayIndexedExpression.position) return if(parent is AssignTarget) { // we're part of the target of an assignment, we have to actually change the assign target itself val memwrite = DirectMemoryWrite(add, arrayIndexedExpression.position) diff --git a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt index f96098c55..5c4e84dc6 100644 --- a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt +++ b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt @@ -128,7 +128,12 @@ internal class StatementReorderer(val program: Program, val errors: IErrorReport } override fun after(arrayIndexedExpression: ArrayIndexedExpression, parent: Node): Iterable { - return replacePointerVarIndexWithMemread(program, arrayIndexedExpression, parent) + if(parent !is VarDecl) { + // don't replace the initializer value in a vardecl - this will be moved to a separate + // assignment statement soon in after(VarDecl) + return replacePointerVarIndexWithMemreadOrMemwrite(program, arrayIndexedExpression, parent) + } + return noModifications } override fun after(expr: BinaryExpression, parent: Node): Iterable { diff --git a/compiler/test/TestSubroutines.kt b/compiler/test/TestSubroutines.kt index 4451dc0c4..33f38d737 100644 --- a/compiler/test/TestSubroutines.kt +++ b/compiler/test/TestSubroutines.kt @@ -4,18 +4,14 @@ import org.junit.jupiter.api.Disabled import org.junit.jupiter.api.Test import org.junit.jupiter.api.TestInstance import prog8.ast.base.DataType -import prog8.ast.expressions.IdentifierReference -import prog8.ast.expressions.TypecastExpression +import prog8.ast.expressions.* import prog8.ast.statements.* import prog8.compiler.target.C64Target import prog8tests.helpers.ErrorReporterForTests import prog8tests.helpers.assertFailure import prog8tests.helpers.assertSuccess import prog8tests.helpers.compileText -import kotlin.test.assertContains -import kotlin.test.assertEquals -import kotlin.test.assertFalse -import kotlin.test.assertTrue +import kotlin.test.* @TestInstance(TestInstance.Lifecycle.PER_CLASS) @@ -181,4 +177,79 @@ class TestSubroutines { assertEquals(DataType.ARRAY_UB, func.parameters.single().type) assertTrue(func.statements.isEmpty()) } + + @Test + fun testUwordParameterAndNormalVarIndexedAsArrayWorkAsDirectMemoryRead() { + val text=""" + main { + sub thing(uword rr) { + ubyte xx = rr[1] ; should still work as var initializer that will be rewritten + ubyte yy + yy = rr[2] + uword other + ubyte zz = other[3] + } + + sub start() { + ubyte[] array=[1,2,3] + thing(array) + } + } + """ + + val result = compileText(C64Target, false, text, writeAssembly = true).assertSuccess() + val module = result.program.toplevelModule + val block = module.statements.single() as Block + val thing = block.statements.filterIsInstance().single {it.name=="thing"} + assertEquals("main", block.name) + assertEquals(10, thing.statements.size, "rr, xx, xx assign, yy, yy assign, other, other assign 0, zz, zz assign, return") + val xx = thing.statements[1] as VarDecl + assertNull(xx.value, "vardecl init values must have been moved to separate assignments") + val assignXX = thing.statements[2] as Assignment + val assignYY = thing.statements[4] as Assignment + val assignZZ = thing.statements[8] as Assignment + assertEquals(listOf("xx"), assignXX.target.identifier!!.nameInSource) + assertEquals(listOf("yy"), assignYY.target.identifier!!.nameInSource) + assertEquals(listOf("zz"), assignZZ.target.identifier!!.nameInSource) + val valueXXexpr = (assignXX.value as DirectMemoryRead).addressExpression as BinaryExpression + val valueYYexpr = (assignYY.value as DirectMemoryRead).addressExpression as BinaryExpression + val valueZZexpr = (assignZZ.value as DirectMemoryRead).addressExpression as BinaryExpression + assertEquals(listOf("rr"), (valueXXexpr.left as IdentifierReference).nameInSource) + assertEquals(listOf("rr"), (valueYYexpr.left as IdentifierReference).nameInSource) + assertEquals(listOf("other"), (valueZZexpr.left as IdentifierReference).nameInSource) + assertEquals(1, (valueXXexpr.right as NumericLiteralValue).number.toInt()) + assertEquals(2, (valueYYexpr.right as NumericLiteralValue).number.toInt()) + assertEquals(3, (valueZZexpr.right as NumericLiteralValue).number.toInt()) + } + + @Test + fun testUwordParameterAndNormalVarIndexedAsArrayWorkAsMemoryWrite() { + val text=""" + main { + sub thing(uword rr) { + rr[10] = 42 + } + + sub start() { + ubyte[] array=[1,2,3] + thing(array) + } + } + """ + + val result = compileText(C64Target, false, text, writeAssembly = true).assertSuccess() + val module = result.program.toplevelModule + val block = module.statements.single() as Block + val thing = block.statements.filterIsInstance().single {it.name=="thing"} + assertEquals("main", block.name) + assertEquals(3, thing.statements.size, "rr, rr assign, return void") + val assignRR = thing.statements[1] as Assignment + assertEquals(42, (assignRR.value as NumericLiteralValue).number.toInt()) + val memwrite = assignRR.target.memoryAddress + assertNotNull(memwrite, "memwrite to set byte array value") + val addressExpr = memwrite.addressExpression as BinaryExpression + assertEquals(listOf("rr"), (addressExpr.left as IdentifierReference).nameInSource) + assertEquals("+", addressExpr.operator) + assertEquals(10, (addressExpr.right as NumericLiteralValue).number.toInt()) + } } diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 7603afd6f..987b042c0 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -6,10 +6,13 @@ TODO For next compiler release (7.2) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ * FIX CRASH BUG * -when compiling petaxian: -Exception in thread "main" prog8.ast.base.FatalAstException: parent node mismatch at IdentifierRef([eRef]) - at prog8.compiler.astprocessing.VariousCleanups.after(VariousCleanups.kt:118) - at prog8.ast.walk.AstWalker.visit(AstWalker.kt:264) +when compiling petaxian: (attack.p8) +Exception in thread "main" prog8.ast.base.FatalAstException: vardecls for variables, with initial numerical value, should have been rewritten as plain vardecl + assignment VarDecl(name=attack_num, vartype=VAR, datatype=UBYTE, value=DirectMemoryRead([IdentifierRef([eRef]) + NumericLiteral(UWORD:11)]), pos=[attack.p8: line 72 col 5-45]) + at prog8.compiler.BeforeAsmGenerationAstChanger.after(BeforeAsmGenerationAstChanger.kt:24) + at prog8.ast.walk.AstWalker.visit(AstWalker.kt:239) + at prog8.ast.statements.VarDecl.accept(AstStatements.kt:248) + + - analyze (and fix?): TODO why are these bigger now than before the var-initializer optimization: ; cube3d-float (THIS ONE IS A LOT BIGGER!!) diff --git a/examples/test.p8 b/examples/test.p8 index 26abf8a46..e009e22cb 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,62 +1,6 @@ -%import textio -%zeropage dontuse +%zeropage basicsafe main { - sub start() { - const ubyte TEST = 10 - ubyte y1 - y1 = TEST + 1 as byte - } -} - -mainzzzzzz { - - - ubyte globalvar1 - ubyte globalvar2 = 99 - - sub start() { - - uword xx0 - - ubyte xx = 100 - uword qq =$1234+xx - - uword ww - ww=$1234+xx - - -; ubyte cv -; sys.memset($1000+xx, 10, 255) ; TODO uses stack eval now to precalc parameters - - ;xx = xx & %0001 ; doesn't use stack... because it uses AugmentableAssignmentAsmGen - ;yy = xx & %0001 ; doesn't use stack... because it uses AugmentableAssignmentAsmGen - - ubyte yy = xx & %0001 ; TODO uses stack eval.... - -; if xx & %0001 { ; TODO why does this use stack? because it uses asmgen.assignExpressionToRegister eventually line 253 in AssignmentAsmGen no augmentable-assignment. -; xx-- -; } - -; if xx+1 { ; TODO why does this use stack? see above -; xx++ -; } -; -; if xx { ; doesn't use stack... -; xx++ -; } -; -; xx = xx+1 ; doesn't use stack... -; -; if 8