From af06fdfffc2c0a35bc063cadc54ce8e7af514ac5 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Thu, 10 Sep 2026 01:36:27 +0200 Subject: [PATCH] fix more 6502-only struct pointer-field bugs; add indexed in-place support fixes #237 --- .../src/prog8/codegen/cpu6502/AsmGen.kt | 2 +- .../codegen/cpu6502/BuiltinFunctionsAsmGen.kt | 77 ++++++++----------- .../cpu6502/assignment/AssignmentAsmGen.kt | 2 +- .../assignment/AugmentableAssignmentAsmGen.kt | 59 +++++++++++++- .../assignment/PointerAssignmentsGen.kt | 15 ++-- .../codegen/intermediate/AssignmentGen.kt | 2 + .../test/codegeneration/TestExecution6502.kt | 55 +++++++++++++ 7 files changed, 157 insertions(+), 55 deletions(-) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt index 4bf5064fa..8d34ea26b 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt @@ -1498,7 +1498,7 @@ $repeatLabel""") } else if(swap.target1.pointerDeref!=null && swap.target2.pointerDeref!=null) { val v1 = swap.target1.pointerDeref!! - val v2 = swap.target1.pointerDeref!! + val v2 = swap.target2.pointerDeref!! if(v1.derefLast && v1.chain.isEmpty() && v2.derefLast && v2.chain.isEmpty() && isZpVar(v1.startpointer) && isZpVar(v2.startpointer)) { // optimized case where v1 and v2 are both already zeropage pointer variables val name1 = asmVariableName(v1.startpointer) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/BuiltinFunctionsAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/BuiltinFunctionsAsmGen.kt index d7ca9d5cc..cf46368ef 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/BuiltinFunctionsAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/BuiltinFunctionsAsmGen.kt @@ -33,8 +33,8 @@ import prog8.codegen.cpu6502.assignment.* return when (fcall.name) { "msw" -> funcMsw(fcall, firstReturnRegister ?: RegisterOrPair.AY) "lsw" -> funcLsw(fcall, firstReturnRegister ?: RegisterOrPair.AY) - "msb" -> funcMsb(fcall, firstReturnRegister ?: RegisterOrPair.A) - "msb__long" -> funcMsbLong(fcall, firstReturnRegister ?: RegisterOrPair.A) + "msb" -> funcMsb(fcall, false, firstReturnRegister ?: RegisterOrPair.A) + "msb__long" -> funcMsb(fcall, true, firstReturnRegister ?: RegisterOrPair.A) "lsb" -> funcLsb(fcall, false, firstReturnRegister ?: RegisterOrPair.A) "lsb__long" -> funcLsb(fcall,true, firstReturnRegister ?: RegisterOrPair.A) "mkword" -> funcMkword(fcall, firstReturnRegister ?: RegisterOrPair.AY) @@ -2426,55 +2426,36 @@ import prog8.codegen.cpu6502.assignment.* return arrayOf(RegisterOrPair.AY) } - private fun funcMsbLong(fcall: PtFunctionCall, resultReg: RegisterOrPair): Array { + private fun funcMsb(fcall: PtFunctionCall, fromLong: Boolean, resultReg: RegisterOrPair): Array { val arg = fcall.args.single() - if (!arg.type.isLong) - throw AssemblyError("msb__long requires long argument") - if (arg is PtNumber) - throw AssemblyError("msb(const) should have been const-folded away") - - if (arg is PtIdentifier) { - val sourceName = asmgen.asmVariableName(arg) - when(resultReg) { - RegisterOrPair.A -> asmgen.out(" lda $sourceName+3") - RegisterOrPair.X -> asmgen.out(" ldx $sourceName+3") - RegisterOrPair.Y -> asmgen.out(" ldy $sourceName+3") - else -> throw AssemblyError("invalid register for msb long: $resultReg ${arg.position}") - } - } else if(arg is PtArrayIndexer) { - TODO("msb of long array element ${fcall.position}") + if(fromLong) { + if (!arg.type.isLong) + throw AssemblyError("msb__long requires long argument") } else { - asmgen.assignExpressionToRegister(arg, RegisterOrPair.R14R15, arg.type.isSigned) - when(resultReg) { - RegisterOrPair.A -> asmgen.out(" lda cx16.r14+3") - RegisterOrPair.X -> asmgen.out(" ldx cx16.r14+3") - RegisterOrPair.Y -> asmgen.out(" ldy cx16.r14+3") - else -> throw AssemblyError("invalid register for msb long: $resultReg ${arg.position}") - } + if (!arg.type.isWord && !arg.type.isPointer) + throw AssemblyError("msb requires word argument") } - - return arrayOf(resultReg) - } - - private fun funcMsb(fcall: PtFunctionCall, resultReg: RegisterOrPair): Array { - val arg = fcall.args.single() - if (!arg.type.isWord) - throw AssemblyError("msb requires word argument") if (arg is PtNumber) throw AssemblyError("msb(const) should have been const-folded away") + + val offset = if(fromLong) 3 else 1 + if (arg is PtIdentifier) { val sourceName = asmgen.asmVariableName(arg) when(resultReg) { - RegisterOrPair.A -> asmgen.out(" lda $sourceName+1") - RegisterOrPair.X -> asmgen.out(" ldx $sourceName+1") - RegisterOrPair.Y -> asmgen.out(" ldy $sourceName+1") - RegisterOrPair.AX -> asmgen.out(" lda $sourceName+1 | ldx #0") - RegisterOrPair.AY -> asmgen.out(" lda $sourceName+1 | ldy #0") - RegisterOrPair.XY -> asmgen.out(" ldx $sourceName+1 | ldy #0") - else -> throw AssemblyError("invalid register for msb: $resultReg ${arg.position}") + RegisterOrPair.A -> asmgen.out(" lda $sourceName+$offset") + RegisterOrPair.X -> asmgen.out(" ldx $sourceName+$offset") + RegisterOrPair.Y -> asmgen.out(" ldy $sourceName+$offset") + RegisterOrPair.AX -> asmgen.out(" lda $sourceName+$offset | ldx #0") + RegisterOrPair.AY -> asmgen.out(" lda $sourceName+$offset | ldy #0") + RegisterOrPair.XY -> asmgen.out(" ldx $sourceName+$offset | ldy #0") + else -> throw AssemblyError("invalid register for msb${if(fromLong) " long" else ""}: $resultReg ${arg.position}") } } else { if(arg is PtArrayIndexer) { + if(fromLong) + TODO("msb of long array element ${fcall.position}") + // just read the msb byte out of the word array if(arg.splitWords) { if(arg.variable==null) @@ -2496,15 +2477,15 @@ import prog8.codegen.cpu6502.assignment.* RegisterOrPair.AX -> { asmgen.loadScaledArrayIndexIntoRegister(arg, CpuRegister.Y) asmgen.out(" lda $arrayVar,y | ldx #0") - } + } RegisterOrPair.AY -> { asmgen.loadScaledArrayIndexIntoRegister(arg, CpuRegister.Y) asmgen.out(" lda $arrayVar,y | ldy #0") - } + } RegisterOrPair.XY -> { asmgen.loadScaledArrayIndexIntoRegister(arg, CpuRegister.X) asmgen.out(" ldy $arrayVar,x | ldx #0") - } + } else -> throw AssemblyError("invalid register for msb: $resultReg ${arg.position}") } } else { @@ -2540,6 +2521,14 @@ import prog8.codegen.cpu6502.assignment.* else -> throw AssemblyError("invalid register for msb: $resultReg ${arg.position}") } } + } else if(fromLong) { + asmgen.assignExpressionToRegister(arg, RegisterOrPair.R14R15, arg.type.isSigned) + when(resultReg) { + RegisterOrPair.A -> asmgen.out(" lda cx16.r14+$offset") + RegisterOrPair.X -> asmgen.out(" ldx cx16.r14+$offset") + RegisterOrPair.Y -> asmgen.out(" ldy cx16.r14+$offset") + else -> throw AssemblyError("invalid register for msb long: $resultReg ${arg.position}") + } } else { asmgen.assignExpressionToRegister(arg, RegisterOrPair.AY) asmgen.out(" tya") @@ -2554,7 +2543,7 @@ import prog8.codegen.cpu6502.assignment.* if(fromLong) { if (!arg.type.isLong) throw AssemblyError("lsb__long requires long") } else { - if (!arg.type.isWord) throw AssemblyError("lsb requires word") + if (!arg.type.isWord && !arg.type.isPointer) throw AssemblyError("lsb requires word") } if (arg is PtNumber) throw AssemblyError("lsb(const) should have been const-folded away") diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt index 5a192d786..f168e4f89 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt @@ -377,7 +377,7 @@ internal class AssignmentAsmGen( throw AssemblyError("weird array element type for pointer indexed read $elementDt") } } else { - val (zpPtrVar, _) = pointergen.deref(pointerDeref, addOffsetToPointer = true) + val (zpPtrVar, _) = pointergen.deref(pointerDeref, addOffsetToPointer = true, loadPointerFieldValue = true) when { elementDt.isByteOrBool -> { diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt index 19d6420a8..1b2453a78 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AugmentableAssignmentAsmGen.kt @@ -62,6 +62,63 @@ internal class AugmentableAssignmentAsmGen(private val program: PtProgram, } } + private fun inplaceModificationIndexedPointerField(target: AsmAssignTarget, operator: String, value: AsmAssignSource) { + val array = target.array!! + val elementDt = target.datatype + if(!elementDt.isByteOrBool && !elementDt.isWord) + throw AssemblyError("inplace modification indexed pointer field for type $elementDt not supported") + val indexedTarget = IndexedPtrTarget(target) + val ptr = ptrgen.indexedEffectiveAddress(indexedTarget) + + val elementTemp = asmgen.createTempVarReused(elementDt.base, false, array) + if(elementDt.isByteOrBool) { + asmgen.out(""" + ldy #0 + lda ($ptr),y + sta $elementTemp""") + } else { + asmgen.out(""" + ldy #0 + lda ($ptr),y + sta $elementTemp + iny + lda ($ptr),y + sta $elementTemp+1""") + } + + // save the computed address on the hardware stack so later code can't clobber it + asmgen.out(""" + lda $ptr + pha + lda $ptr+1 + pha""") + + // reuse the existing variable-target logic to modify the temp + val tempTarget = AsmAssignTarget(TargetStorageKind.VARIABLE, asmgen, elementDt, null, target.position, variableAsmName = elementTemp) + inplaceModification(tempTarget, operator, value) + + // restore the address and store the temp back + asmgen.out(""" + pla + sta $ptr+1 + pla + sta $ptr""") + if(elementDt.isByteOrBool) { + asmgen.out(""" + ldy #0 + lda $elementTemp + sta ($ptr),y""") + } else { + asmgen.out(""" + ldy #0 + lda $elementTemp + sta ($ptr),y + iny + lda $elementTemp+1 + sta ($ptr),y""") + } + } + private fun inplaceModification(target: AsmAssignTarget, operator: String, value: AsmAssignSource) { // the asm-gen code can deal with situations where you want to assign a byte into a word. @@ -247,7 +304,7 @@ internal class AugmentableAssignmentAsmGen(private val program: PtProgram, TargetStorageKind.ARRAY -> { val deref = target.array!!.pointerderef if(deref!=null) { - TODO("inplace modification array indexed pointer deref ${target.position}") + inplaceModificationIndexedPointerField(target, operator, value) return } val targetArrayVar = target.array.variable!! diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/PointerAssignmentsGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/PointerAssignmentsGen.kt index 3e71ca2b4..a3106c2d5 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/PointerAssignmentsGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/PointerAssignmentsGen.kt @@ -228,10 +228,11 @@ internal class PointerAssignmentsGen(private val asmgen: AsmGen6502Internal, pri } - internal fun deref(pointer: PtPointerDeref, addOffsetToPointer: Boolean=false): Pair { + internal fun deref(pointer: PtPointerDeref, addOffsetToPointer: Boolean=false, loadPointerFieldValue: Boolean=false): Pair { // walk the pointer deref chain and leaves the final pointer value in a ZP var // this will often be the temp var P8ZP_SCRATCH_PTR but can also be the original pointer variable if it is already in zeropage and there is nothing to add to it // returns the ZP var to use as a pointer, and a Y register offset (which can be zero) + // loadPointerFieldValue: for addOffsetToPointer=true, also load the value if the final field is a pointer-typed field (rather than returning its address). fun addFieldOffsetToScratchPointer(fieldoffset: UInt) { if(fieldoffset==0u) @@ -276,8 +277,7 @@ internal class PointerAssignmentsGen(private val asmgen: AsmGen6502Internal, pri if(addOffsetToPointer) { asmgen.assignExpressionToVariable(pointer.startpointer, "P8ZP_SCRATCH_PTR", DataType.UWORD) addFieldOffsetToScratchPointer(field.second.toUInt()) - // if the target field is a pointer (or array indexed through a pointer), use its value as the base address - if(pointer.derefLast || field.first.isPointer) + if(pointer.derefLast || (loadPointerFieldValue && field.first.isPointer)) updateScratchPointer() return "P8ZP_SCRATCH_PTR" to 0u } else { @@ -327,8 +327,7 @@ internal class PointerAssignmentsGen(private val asmgen: AsmGen6502Internal, pri } else { if(addOffsetToPointer) { addFieldOffsetToScratchPointer(fieldinfo.second.toUInt()) - // if the target field is a pointer, use its value as the base address - if(fieldinfo.first.isPointer) + if(loadPointerFieldValue && fieldinfo.first.isPointer) updateScratchPointer() return "P8ZP_SCRATCH_PTR" to 0u } else @@ -361,7 +360,7 @@ internal class PointerAssignmentsGen(private val asmgen: AsmGen6502Internal, pri } // save scaled index to W2, compute base (field-corrected) into P8ZP_SCRATCH_PTR, then add asmgen.out(" lda P8ZP_SCRATCH_W1 | sta P8ZP_SCRATCH_W2 | lda P8ZP_SCRATCH_W1+1 | sta P8ZP_SCRATCH_W2+1") - val (basePtr, _) = deref(target.pointer, addOffsetToPointer = true) + val (basePtr, _) = deref(target.pointer, addOffsetToPointer = true, loadPointerFieldValue = true) if(basePtr != "P8ZP_SCRATCH_PTR") { // deref returned the original zp pointer variable; copy it so we don't mutate it asmgen.out(""" @@ -454,7 +453,7 @@ internal class PointerAssignmentsGen(private val asmgen: AsmGen6502Internal, pri inplaceByteInc(target, amount) } else { if (target.dt.isByte) inplaceByteAdd(target, value) - else if (target.dt.isWord) inplaceWordAdd(target, value) + else if (target.dt.isWord || target.dt.isPointer) inplaceWordAdd(target, value) else if (target.dt.isFloat) inplaceFloatAddOrMul(target, "FADD", value) else if (target.dt.isLong) inplaceLongAdd(target, value) else throw AssemblyError("weird dt ${target.dt} ${target.position}") @@ -466,7 +465,7 @@ internal class PointerAssignmentsGen(private val asmgen: AsmGen6502Internal, pri inplaceByteDec(target, amount) } else { if (target.dt.isByte) inplaceByteSub(target, value) - else if (target.dt.isWord) inplaceWordSub(target, value) + else if (target.dt.isWord || target.dt.isPointer) inplaceWordSub(target, value) else if (target.dt.isFloat) inplaceFloatSubOrDiv(target, "FSUB", value) else if (target.dt.isLong) inplaceLongSub(target, value) else throw AssemblyError("weird dt ${target.position}") diff --git a/codeGenIntermediate/src/prog8/codegen/intermediate/AssignmentGen.kt b/codeGenIntermediate/src/prog8/codegen/intermediate/AssignmentGen.kt index 8cf2bf8ed..17a3cc658 100644 --- a/codeGenIntermediate/src/prog8/codegen/intermediate/AssignmentGen.kt +++ b/codeGenIntermediate/src/prog8/codegen/intermediate/AssignmentGen.kt @@ -253,6 +253,8 @@ internal class AssignmentGen(private val codeGen: IRCodeGen, private val exprGen codeGen.storeValueAtPointersLocation(inplaceInstrs, addressReg, fieldOffset, pointerDeref.type, false, oldvalueReg) chunks = inplaceInstrs + } else if(array?.pointerderef != null) { + chunks = fallbackAssign(augAssign) } else { chunks = when (augAssign.operator) { "+=" -> operatorPlusInplace(symbol, array, constAddress, memTarget, targetDt, value) diff --git a/compiler/test/codegeneration/TestExecution6502.kt b/compiler/test/codegeneration/TestExecution6502.kt index 74c99dcbd..516aeb226 100644 --- a/compiler/test/codegeneration/TestExecution6502.kt +++ b/compiler/test/codegeneration/TestExecution6502.kt @@ -117,6 +117,61 @@ class TestExecution6502 : FunSpec({ machine.assertMemory(4500+5, 1234 shr 8) } + test("more struct pointer field operations") { + val src = $$""" + %option no_sysinit + %launcher none + %address $1000 + %zeropage basicsafe + + main { + struct Node { + bool flag + ^^uword uptr + } + ^^Node @shared n = [true, 4000] + ^^Node @shared m = [true, 5000] + ^^uword @shared normalptr = 4000 + &ubyte poweroff = $f203 + &uword n_after_swap = $0300 + &uword m_after_swap = $0302 + &uword n_after_add = $0304 + &uword lsb_result = $0306 + &uword msb_result = $0308 + &uword idxinc_result = $030A + + sub start() { + swap(n.uptr, m.uptr) + n_after_swap = n.uptr + m_after_swap = m.uptr + n.uptr += 2 + n_after_add = n.uptr + n.uptr[3] = 100 + n.uptr[3]++ + idxinc_result = n.uptr[3] + lsb_result = lsb(normalptr) + msb_result = msb(normalptr) + poweroff = 1 + } + } + """.trimIndent() + + val compileResult = compileText(Cx16Target(), false, src, outputDir) + val machine = compileResult!!.simulate() + machine.assertMemory(0x0300, 5000 and 0xff) + machine.assertMemory(0x0301, 5000 shr 8) + machine.assertMemory(0x0302, 4000 and 0xff) + machine.assertMemory(0x0303, 4000 shr 8) + machine.assertMemory(0x0304, 5004 and 0xff) + machine.assertMemory(0x0305, 5004 shr 8) + machine.assertMemory(0x0306, 160) + machine.assertMemory(0x0308, 15) + machine.assertMemory(0x030A, 101) + machine.assertMemory(0x030B, 0) + machine.assertMemory(5010, 101) + machine.assertMemory(5011, 0) + } + test("serial output") { val src = $$""" %encoding iso