Fix 6502 codegen for pointer-to-pointer struct field double-deref

cherry-picked from master e74c34a5

codeCore/src/prog8/code/core/ICompilationTarget.kt

@@ -44,6 +44,11 @@ enum class CpuType(val statusBitsOnMultiByteOps: Boolean) {
      * behavior consistent across all current targets.
      */
     VIRTUAL(false)
+    ;
+
+    /** True for the 6502-family targets (plain 6502 and 65C02). */
+    val is6502: Boolean
+        get() = this == CPU6502 || this == CPU65C02
 }
 
 interface ICompilationTarget: IStringEncoding, IMemSizer {

codeGenCpu6502/src/prog8/codegen/cpu6502/BuiltinFunctionsAsmGen.kt

@@ -20,6 +20,25 @@ import prog8.codegen.cpu6502.assignment.*
         translateFunctioncall(fcall, null, discardResult = true)
     }
 
+    /**
+     * Several builtins can only produce their result in a CPU register, and which one that is
+     * depends on the width of the result: the byte-returning ones (lsb/msb) take a single register,
+     * the word-returning ones (lsw/msw) and popw take a register pair, and the builtin picks the
+     * specific register. [hardwareReg] is the register to fall back on when the caller has no
+     * preference, or when it asked for something the builtin cannot deliver to directly.
+     *
+     * The caller's requested register is honoured when it is a real CPU register. A CX16 virtual
+     * register (for example an asmsub parameter declared as @R0) is not: those are memory locations,
+     * so the builtin produces its result in [hardwareReg] and the caller's assignment then stores it
+     * into the virtual register. Passing the virtual register through would make the builtin reject
+     * it as an invalid result register.
+     *
+     * (funcPeekW and funcMkword already handle virtual registers themselves.)
+     */
+    private fun withResultRegister(requested: RegisterOrPair?, hardwareReg: RegisterOrPair, compute: (RegisterOrPair) -> Array<RegisterOrPair>): Array<RegisterOrPair> =
+        if (requested != null && requested !in Cx16VirtualRegisters) compute(requested)
+        else compute(hardwareReg)
+
     private fun translateFunctioncall(fcall: PtFunctionCall, firstReturnRegister: RegisterOrPair?, discardResult: Boolean): Array<RegisterOrPair> {
         
         // Note: returns the actual register(s) that the return value is in, so the caller should still make sure to place it into the actual correct destination.
@@ -31,12 +50,12 @@ import prog8.codegen.cpu6502.assignment.*
         val sscope = fcall.definingISub()
 
         return when (fcall.name) {
-            "msw" -> funcMsw(fcall, firstReturnRegister ?: RegisterOrPair.AY)
-            "lsw" -> funcLsw(fcall, firstReturnRegister ?: RegisterOrPair.AY)
-            "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)
+            "msw" -> withResultRegister(firstReturnRegister, RegisterOrPair.AY) { funcMsw(fcall, it) }
+            "lsw" -> withResultRegister(firstReturnRegister, RegisterOrPair.AY) { funcLsw(fcall, it) }
+            "msb" -> withResultRegister(firstReturnRegister, RegisterOrPair.A) { funcMsb(fcall, false, it) }
+            "msb__long" -> withResultRegister(firstReturnRegister, RegisterOrPair.A) { funcMsb(fcall, true, it) }
+            "lsb" -> withResultRegister(firstReturnRegister, RegisterOrPair.A) { funcLsb(fcall, false, it) }
+            "lsb__long" -> withResultRegister(firstReturnRegister, RegisterOrPair.A) { funcLsb(fcall, true, it) }
             "mkword" -> funcMkword(fcall, firstReturnRegister ?: RegisterOrPair.AY)
             "mklong", "mklong2" -> funcMklong(fcall, firstReturnRegister ?: RegisterOrPair.R14R15)
             "clamp__byte", "clamp__ubyte" -> funcClamp(fcall)
@@ -98,8 +117,8 @@ import prog8.codegen.cpu6502.assignment.*
             "pushw" -> funcPushW(fcall)
             "pushl" -> funcPushL(fcall)
             "pushf" -> funcPushF(fcall)
-            "pop" -> funcPop(firstReturnRegister ?: RegisterOrPair.A)
-            "popw" -> funcPopW(firstReturnRegister ?: RegisterOrPair.AY)
+            "pop" -> withResultRegister(firstReturnRegister, RegisterOrPair.A) { funcPop(it) }
+            "popw" -> withResultRegister(firstReturnRegister, RegisterOrPair.AY) { funcPopW(it) }
             "popl" -> funcPopL()
             "popf" -> funcPopF()
             else -> throw AssemblyError("sizeof must have been replaced with a constant  ${fcall.position}")

codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/PointerAssignmentsGen.kt

@@ -228,11 +228,19 @@ internal class PointerAssignmentsGen(private val asmgen: AsmGen6502Internal, pri
     }
 
 
+    /**
+     * Walk the pointer deref chain. Always returns (zpVar, offset) such that one indirect
+     * access through zpVar at byte offset reaches the final thing:
+     *   derefLast=true  -> (zpVar),offset is the address of the value itself
+     *   derefLast=false -> (zpVar),offset is the location of the field content
+     * This contract holds for every chain length: the final pointer (if any) is already
+     * followed inside deref(), so callers never need a second dereference.
+     * zpVar is P8ZP_SCRATCH_PTR for non-empty chains or non-zeropage pointers; it may be
+     * the original (zeropage) pointer variable only for an empty chain with derefLast=false.
+     * loadPointerFieldValue: for addOffsetToPointer=true, also load the value if the final
+     * field is a pointer-typed field (rather than returning its address).
+     */
     internal fun deref(pointer: PtPointerDeref, addOffsetToPointer: Boolean=false, loadPointerFieldValue: Boolean=false): Pair<String, UByte> {
-        // 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)
@@ -285,6 +293,7 @@ internal class PointerAssignmentsGen(private val asmgen: AsmGen6502Internal, pri
                     if(pointer.derefLast) {
                         asmgen.assignExpressionToVariable(pointer.startpointer, "P8ZP_SCRATCH_PTR", DataType.UWORD)
                         addFieldOffsetToScratchPointer(field.second.toUInt())
+                        updateScratchPointer()
                         return "P8ZP_SCRATCH_PTR" to 0u
                     }
                     else
@@ -295,6 +304,7 @@ internal class PointerAssignmentsGen(private val asmgen: AsmGen6502Internal, pri
                     asmgen.assignExpressionToVariable(pointer.startpointer, "P8ZP_SCRATCH_PTR", DataType.UWORD)
                     if(pointer.derefLast) {
                         addFieldOffsetToScratchPointer(field.second.toUInt())
+                        updateScratchPointer()
                         return "P8ZP_SCRATCH_PTR" to 0u
                     }
                     else
@@ -381,15 +391,7 @@ internal class PointerAssignmentsGen(private val asmgen: AsmGen6502Internal, pri
     }
 
     internal fun assignPointerDerefExpression(target: AsmAssignTarget, value: PtPointerDeref) {
-        val (zpPtrVar, offset1) = deref(value)
-        val offset: UByte
-        if(value.derefLast) {
-            asmgen.loadIndirectWordAY(zpPtrVar, offset1)
-            asmgen.out("  sta  $zpPtrVar |  sty  $zpPtrVar+1")
-            offset = 0.toUByte()
-        } else {
-            offset = offset1
-        }
+        val (zpPtrVar, offset) = deref(value)
         if(value.type.isByteOrBool) {
             asmgen.loadIndirectByte(zpPtrVar, offset)
             asmgen.assignRegister(RegisterOrPair.A, target)

codeGenIntermediate/src/prog8/codegen/intermediate/BuiltinFuncGen.kt

@@ -120,21 +120,21 @@ internal class BuiltinFuncGen(private val codeGen: IRCodeGen, private val exprGe
         val result = mutableListOf<IRCodeChunkBase>()
         val resultRegister = codeGen.registers.next(IRDataType.WORD)
         addInstr(result, IRInstruction(Opcode.POP, IRDataType.WORD, reg1=resultRegister), null)
-        return ExpressionCodeResult(result, IRDataType.BYTE, resultRegister, -1)
+        return ExpressionCodeResult(result, IRDataType.WORD, resultRegister, -1)
     }
 
     private fun funcPopL(): ExpressionCodeResult {
         val result = mutableListOf<IRCodeChunkBase>()
         val resultRegister = codeGen.registers.next(IRDataType.LONG)
         addInstr(result, IRInstruction(Opcode.POP, IRDataType.LONG, reg1=resultRegister), null)
-        return ExpressionCodeResult(result, IRDataType.BYTE, resultRegister, -1)
+        return ExpressionCodeResult(result, IRDataType.LONG, resultRegister, -1)
     }
 
     private fun funcPopF(): ExpressionCodeResult {
         val result = mutableListOf<IRCodeChunkBase>()
         val resultRegister = codeGen.registers.next(IRDataType.FLOAT)
         addInstr(result, IRInstruction(Opcode.POP, IRDataType.FLOAT, fpReg1 = RegisterNum(resultRegister)), null)
-        return ExpressionCodeResult(result, IRDataType.BYTE, -1, resultRegister)
+        return ExpressionCodeResult(result, IRDataType.FLOAT, -1, resultRegister)
     }
 
     private fun funcCopyFromPointer1ToPointer2(call: PtFunctionCall, type: IRDataType): ExpressionCodeResult {

codeGenIntermediate/src/prog8/codegen/intermediate/ExpressionGen.kt

@@ -22,6 +22,21 @@ internal class ExpressionCodeResult(val chunks: IRCodeChunks, val dt: IRDataType
     }
 }
 
+/**
+ * True if the explicit CMPI #0 that would test a translated result value against zero can be
+ * skipped, because the last emitted instruction of that translation already sets the status
+ * bits. This is only allowed on targets that honor the "multi-byte ops set status bits based
+ * on the full value" contract (see CpuType.statusBitsOnMultiByteOps): on 8-bit targets,
+ * multi-byte ops may only set Z from the last byte, so the explicit CMPI is required there.
+ *
+ * Note: when the CMPI is skipped, a following branch tests Z meaning "the compared value is
+ * zero". That is only meaningful when the last instruction was itself a CMP/CMPI/SGN/BITTST
+ * (what OpcodesThatSetStatusbits guarantees). A future target that honors the contract while
+ * setting status bits with other instructions must re-check this assumption.
+ */
+internal fun CpuType.canSkipExplicitCmpi(lastInstr: IRInstruction?): Boolean =
+        statusBitsOnMultiByteOps && lastInstr != null && lastInstr.opcode in OpcodesThatSetStatusbits
+
 internal class ExpressionGen(private val codeGen: IRCodeGen) {
 
     fun translateExpression(expr: PtExpression): ExpressionCodeResult {
@@ -268,14 +283,8 @@ internal class ExpressionGen(private val codeGen: IRCodeGen) {
         // Use status flags from condition expression when possible, avoiding redundant CMPI #0
         val condTr = translateExpression(ifExpr.condition)
         addToResult(result, condTr, condTr.resultReg, -1)
-        // Only skip the CMPI if the target honors the "multi-byte ops set Z based on full
-        // value" contract. On 8-bit targets (6502/65C02) this is false, so we always emit
-        // an explicit CMPI - a multi-byte op may have set Z based on only the last byte.
-        // See CpuType.statusBitsOnMultiByteOps for the rationale.
         val lastInstr = condTr.chunks.lastOrNull()?.instructions?.lastOrNull()
-        val skipCmpi = codeGen.options.compTarget.cpu.statusBitsOnMultiByteOps
-                && lastInstr != null
-                && lastInstr.opcode in OpcodesThatSetStatusbits
+        val skipCmpi = codeGen.options.compTarget.cpu.canSkipExplicitCmpi(lastInstr)
         if (!skipCmpi) {
             addInstr(result, IRInstruction(Opcode.CMPI, IRDataType.BYTE, reg1=condTr.resultReg, immediate = 0), null)
         }
@@ -1316,14 +1325,8 @@ internal class ExpressionGen(private val codeGen: IRCodeGen) {
                     val leftTr = translateExpression(binExpr.left)
                     addToResult(result, leftTr, leftTr.resultReg, -1)
                     val lastInstr = leftTr.chunks.lastOrNull()?.instructions?.lastOrNull()
-                    // Only skip the CMPI if the target honors the "multi-byte ops set Z
-                    // based on full value" contract. On 8-bit targets a previous op like
-                    // DEC only set Z from the last byte, so the explicit CMPI is required.
-                    // See CpuType.statusBitsOnMultiByteOps for the rationale.
                     val skipCmpi = rightConst.toInt() == 0
-                            && codeGen.options.compTarget.cpu.statusBitsOnMultiByteOps
-                            && lastInstr != null
-                            && lastInstr.opcode in OpcodesThatSetStatusbits
+                            && codeGen.options.compTarget.cpu.canSkipExplicitCmpi(lastInstr)
                     if (!skipCmpi) {
                         addInstr(result, IRInstruction(Opcode.CMPI, leftTr.dt, reg1 = leftTr.resultReg, immediate = rightConst.toInt()), null)
                     }
@@ -1450,6 +1453,25 @@ internal class ExpressionGen(private val codeGen: IRCodeGen) {
         }
     }
 
+    /**
+     * Emit the branch that decides if the short-circuit evaluation of a boolean `or`/`and` has to
+     * skip its right hand side: [branchWhenZeroTrue] selects if the branch is taken when the left
+     * hand side value in the register is zero (false) instead of when it is non-zero (true).
+     * The status bits have to be set from that register first, because only CMP/CMPI/SGN/BITTST do
+     * that on targets that don't honor CpuType.statusBitsOnMultiByteOps, and the value can also come
+     * straight from a load or a register move. [canSkipExplicitCmpi] decides if setting them can be
+     * skipped on this target.
+     */
+    private fun addShortCircuitBranch(result: MutableList<IRCodeChunkBase>, leftTr: ExpressionCodeResult, shortcutLabel: String, branchWhenZeroTrue: Boolean) {
+        val lastInstr = leftTr.chunks.lastOrNull()?.instructions?.lastOrNull()
+        val skipCmpi = codeGen.options.compTarget.cpu.canSkipExplicitCmpi(lastInstr)
+        if (!skipCmpi) {
+            addInstr(result, IRInstruction(Opcode.CMPI, IRDataType.BYTE, reg1 = leftTr.resultReg, immediate = 0), null)
+        }
+        val branchOpcode = if (branchWhenZeroTrue) Opcode.BSTEQ else Opcode.BSTNE
+        addInstr(result, IRInstruction(branchOpcode, labelSymbol = shortcutLabel), null)
+    }
+
     private fun operatorAnd(binExpr: PtBinaryExpression, vmDt: IRDataType, bitwise: Boolean): ExpressionCodeResult {
         val result = mutableListOf<IRCodeChunkBase>()
         if(!bitwise && !binExpr.right.isSimple()) {
@@ -1457,7 +1479,7 @@ internal class ExpressionGen(private val codeGen: IRCodeGen) {
             val leftTr = translateExpression(binExpr.left)
             addToResult(result, leftTr, leftTr.resultReg, -1)
             val shortcutLabel = codeGen.createLabelName()
-            addInstr(result, IRInstruction(Opcode.BSTEQ, labelSymbol = shortcutLabel), null)
+            addShortCircuitBranch(result, leftTr, shortcutLabel, branchWhenZeroTrue = true)
             val rightTr = translateExpression(binExpr.right)
             addToResult(result, rightTr, leftTr.resultReg, -1)
             result += IRCodeChunk(shortcutLabel, null)
@@ -1491,7 +1513,7 @@ internal class ExpressionGen(private val codeGen: IRCodeGen) {
             val leftTr = translateExpression(binExpr.left)
             addToResult(result, leftTr, leftTr.resultReg, -1)
             val shortcutLabel = codeGen.createLabelName()
-            addInstr(result, IRInstruction(Opcode.BSTNE, labelSymbol = shortcutLabel), null)
+            addShortCircuitBranch(result, leftTr, shortcutLabel, branchWhenZeroTrue = false)
             val rightTr = translateExpression(binExpr.right)
             addToResult(result, rightTr, leftTr.resultReg, -1)
             result += IRCodeChunk(shortcutLabel, null)

codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt

@@ -888,7 +888,10 @@ class IRCodeGen(
         val chunk2 = addConstMem(loopvarDtIr, null, loopvarSymbol, iterable.step)
         if(loopvarDtIr==IRDataType.BYTE && iterable.step==-1 && iterable.last==0) {
             // downto 0 optimization (byte)
-            if(loopvarDt.isSignedByte || iterable.first<=127) {
+            // Only rely on DECM setting status bits on targets where it actually does (6502 hardware,
+            // or targets that honor the strict multi-byte status-bits contract). On the VM DECM
+            // leaves flags untouched, so BSTPOS would read stale state and loop forever.
+            if((loopvarDt.isSignedByte || iterable.first<=127) && (options.compTarget.cpu.is6502 || options.compTarget.cpu.statusBitsOnMultiByteOps)) {
                 chunk2 += IRInstruction(Opcode.BSTPOS, labelSymbol = loopLabel)
             } else {
                 chunk2 += IRInstruction(Opcode.LOADM, loopvarDtIr, reg1 = indexReg, labelSymbol = loopvarSymbol)
@@ -1336,17 +1339,8 @@ class IRCodeGen(
         // Fallback: materialize expression and branch on result
         val tr = expressionEval.translateExpression(condition)
         result += tr.chunks
-        // Only skip the CMPI #0 if BOTH:
-        //   1) The previous instruction is documented to set status bits (Z/N), AND
-        //   2) The target CPU honors the "multi-byte ops set Z based on full value" contract
-        //      (see CpuType.statusBitsOnMultiByteOps). For 8-bit targets this contract
-        //      is false: e.g. a 16-bit DEC only sets Z from the low byte's dec, not the
-        //      full 16-bit value, so we MUST emit an explicit CMPI here.
         val lastInstr = tr.chunks.lastOrNull()?.instructions?.lastOrNull()
-        val targetHonorsContract = options.compTarget.cpu.statusBitsOnMultiByteOps
-        val skipCmpi = targetHonorsContract
-                && lastInstr != null
-                && lastInstr.opcode in OpcodesThatSetStatusbits
+        val skipCmpi = options.compTarget.cpu.canSkipExplicitCmpi(lastInstr)
         if (!skipCmpi) {
             addInstr(result, IRInstruction(Opcode.CMPI, tr.dt, reg1 = tr.resultReg, immediate = 0), null)
         }
@@ -1389,15 +1383,8 @@ class IRCodeGen(
         if (number != null) {
             val isComparingWithZero = number == 0
             val lastInstr = leftTr.chunks.lastOrNull()?.instructions?.lastOrNull()
-            // Skip the CMPI only if the target honors the "multi-byte ops set Z correctly"
-            // contract. On 8-bit targets a 16/32-bit op like DEC only sets Z from the last
-            // byte, so we cannot rely on the previous instruction's status bits - the
-            // CMPI is required to test the full multi-byte value.
-            // See CpuType.statusBitsOnMultiByteOps for details.
             val canSkipCmpi = isComparingWithZero
-                    && options.compTarget.cpu.statusBitsOnMultiByteOps
-                    && lastInstr != null
-                    && lastInstr.opcode in OpcodesThatSetStatusbits
+                    && options.compTarget.cpu.canSkipExplicitCmpi(lastInstr)
 
             if ((onTrueLabel != null || onTrueAddress != null) && (onFalseLabel == null && onFalseAddress == null)) {
                 var (opcode, useCmpi) = getIntegerComparisonBranch(condition.operator, false, signed)

codeOptimizers/src/prog8/optimizer/StatementOptimizer.kt

@@ -494,15 +494,19 @@ class StatementOptimizer(private val program: Program,
                     return listOf(AstReplaceNode(assignment, ifstmt, parent))
                 }
 
-                if(v2 is NumericLiteral || v2 is IdentifierReference) {
+                if(v2 is NumericLiteral) {
                     // x = min(expression, 100)  ->  x=expression,  if x>100  x=100
+                    // Only safe with a real constant: the assignment to x happens before the
+                    // comparison, so comparing against anything that x could alias (such as a
+                    // variable that happens to be the assignment target) would read the already
+                    // overwritten value.
                     val assign = Assignment(assignment.target.copy(), v1, AssignmentOrigin.OPTIMIZER, assignment.position)
                     val ifstmt = makeMinMaxCheckAndAssignRight(assign.target.toExpression(), ">", v2, assignment.target, assignment.position)
                     return listOf(
                         AstInsert.after(assignment, ifstmt, parent as IStatementContainer),
                         AstReplaceNode(assignment, assign, parent)
                     )
-                } else if(v1 is NumericLiteral || v1 is IdentifierReference) {
+                } else if(v1 is NumericLiteral) {
                     // x = min(100, expression)  ->  x=expression,  if x>100  x=100
                     val assign = Assignment(assignment.target.copy(), v2, AssignmentOrigin.OPTIMIZER, assignment.position)
                     val ifstmt = makeMinMaxCheckAndAssignRight(assign.target.toExpression(), ">", v1, assignment.target, assignment.position)
@@ -527,15 +531,17 @@ class StatementOptimizer(private val program: Program,
                     return listOf(AstReplaceNode(assignment, ifstmt, parent))
                 }
 
-                if(v2 is NumericLiteral || v2 is IdentifierReference) {
+                if(v2 is NumericLiteral) {
                     // x = max(expression, 100)  ->  x=expression,  if x<100  x=100
+                    // See the note in the min__ case above: the compared operand must be a
+                    // real constant, never a variable that the assignment could have aliased.
                     val assign = Assignment(assignment.target.copy(), v1, AssignmentOrigin.OPTIMIZER, assignment.position)
                     val ifstmt = makeMinMaxCheckAndAssignRight(assign.target.toExpression(), "<", v2, assignment.target, assignment.position)
                     return listOf(
                         AstInsert.after(assignment, ifstmt, parent as IStatementContainer),
                         AstReplaceNode(assignment, assign, parent)
                     )
-                } else if(v1 is NumericLiteral || v1 is IdentifierReference) {
+                } else if(v1 is NumericLiteral) {
                     // x = max(100, expression)  ->  x=expression,  if x<100  x=100
                     val assign = Assignment(assignment.target.copy(), v2, AssignmentOrigin.OPTIMIZER, assignment.position)
                     val ifstmt = makeMinMaxCheckAndAssignRight(assign.target.toExpression(), "<", v1, assignment.target, assignment.position)

compiler/test/TestCompilerOnExamples.kt

@@ -331,16 +331,16 @@ class TestCompilerOnExamplesVirtual: FunSpec({
     val onlyVirtual = cartesianProduct(
         listOf(
             ExampleSizes("bouncegfx", virtualInstrCountOptimized=240, virtualInstrCountUnoptimized=856, virtualRegCountOptimized=97, virtualRegCountUnoptimized=386),
-            ExampleSizes("bsieve", virtualInstrCountOptimized=273, virtualInstrCountUnoptimized=2001, virtualRegCountOptimized=74, virtualRegCountUnoptimized=866),
+            ExampleSizes("bsieve", virtualInstrCountOptimized=273, virtualInstrCountUnoptimized=2011, virtualRegCountOptimized=74, virtualRegCountUnoptimized=866),
             ExampleSizes("fountain", virtualInstrCountOptimized=207, virtualInstrCountUnoptimized=824, virtualRegCountOptimized=80, virtualRegCountUnoptimized=369),
             ExampleSizes("pixelshader", virtualInstrCountOptimized=93, virtualInstrCountUnoptimized=308, virtualRegCountOptimized=17, virtualRegCountUnoptimized=115),
             ExampleSizes("sincos", virtualInstrCountOptimized=335, virtualInstrCountUnoptimized=930, virtualRegCountOptimized=138, virtualRegCountUnoptimized=409),
-            ExampleSizes("pointers/animalgame", virtualInstrCountOptimized=330, virtualInstrCountUnoptimized=2145, virtualRegCountOptimized=111, virtualRegCountUnoptimized=939),
-            ExampleSizes("pointers/binarytree", virtualInstrCountOptimized=658, virtualInstrCountUnoptimized=2327, virtualRegCountOptimized=260, virtualRegCountUnoptimized=1026),
-            ExampleSizes("pointers/hashtable", virtualInstrCountOptimized=700, virtualInstrCountUnoptimized=2326, virtualRegCountOptimized=300, virtualRegCountUnoptimized=1051),
-            ExampleSizes("pointers/sortedlist", virtualInstrCountOptimized=404, virtualInstrCountUnoptimized=2488, virtualRegCountOptimized=130, virtualRegCountUnoptimized=1092),
+            ExampleSizes("pointers/animalgame", virtualInstrCountOptimized=330, virtualInstrCountUnoptimized=2155, virtualRegCountOptimized=111, virtualRegCountUnoptimized=939),
+            ExampleSizes("pointers/binarytree", virtualInstrCountOptimized=658, virtualInstrCountUnoptimized=2333, virtualRegCountOptimized=260, virtualRegCountUnoptimized=1026),
+            ExampleSizes("pointers/hashtable", virtualInstrCountOptimized=700, virtualInstrCountUnoptimized=2336, virtualRegCountOptimized=300, virtualRegCountUnoptimized=1051),
+            ExampleSizes("pointers/sortedlist", virtualInstrCountOptimized=404, virtualInstrCountUnoptimized=2494, virtualRegCountOptimized=130, virtualRegCountUnoptimized=1092),
             ExampleSizes("pointers/fountain-virtual", virtualInstrCountOptimized=203, virtualInstrCountUnoptimized=819, virtualRegCountOptimized=75, virtualRegCountUnoptimized=363),
-            ExampleSizes("pointers/sorting", virtualInstrCountOptimized=530, virtualInstrCountUnoptimized=2322, virtualRegCountOptimized=183, virtualRegCountUnoptimized=1016)
+            ExampleSizes("pointers/sorting", virtualInstrCountOptimized=530, virtualInstrCountUnoptimized=2330, virtualRegCountOptimized=183, virtualRegCountUnoptimized=1016)
         ),
         listOf(false, true)
     )

compiler/test/TestOptimization.kt

@@ -880,6 +880,107 @@ main {
         ifStmt.condition shouldBe instanceOf<BinaryExpression>()
     }
 
+    // The four cases below each cover one of the four "split into assign + compare" rewrite
+    // sites in StatementOptimizer (min/max x either operand order). All of them assign to the
+    // target before the comparison, so the compared operand must be a real constant.
+    fun assertNotReducedToOneOperand(source: String, expectedFuncPrefix: String) {
+        val result = compileText(Cx16Target(), optimize = true, source, outputDir, writeAssembly = false)!!
+        val stmts = result.compilerAst.entrypoint.statements
+        // the assignment under test is the last one before the implicit return
+        val assignment = stmts.filterIsInstance<Assignment>().last()
+        val call = assignment.value as? FunctionCallExpression
+        withClue("expected the ${expectedFuncPrefix}() call to survive, got: ${assignment.value}") {
+            call shouldNotBe null
+        }
+        withClue("expected a ${expectedFuncPrefix}__* builtin call") {
+            call!!.target.nameInSource.single() shouldStartWith expectedFuncPrefix
+        }
+    }
+
+    test("min/max assignment where the target aliases an operand keeps the comparison") {
+        // The rewrite of "x = min(expr, c)" into "x = expr; if x > c then x = c" assigns to x
+        // before doing the comparison, so it is only valid when the compared operand is a real
+        // constant. When it is a variable that aliases x, the comparison reads the already
+        // overwritten value and the min/max silently degenerates into plain "x = expr".
+        // See github issue #247.
+        assertNotReducedToOneOperand("""
+        main {
+            word @shared err
+            word @shared speed
+            sub start() {
+                err = 80
+                speed = 1
+                speed = min(err / 8, speed)
+            }
+        }""", "min")
+    }
+
+    test("min/max split into assign+compare is still applied for constant operands") {
+        val src = """
+        main {
+            word @shared x
+            sub start() {
+                x = 5
+                x = min(x, 100)
+            }
+        }"""
+        val result = compileText(Cx16Target(), optimize = true, src, outputDir, writeAssembly = false)!!
+        val stmts = result.compilerAst.entrypoint.statements
+        stmts.any { it is IfElse } shouldBe true
+    }
+
+    test("min/max keeps comparison when target aliases the second operand") {
+        assertNotReducedToOneOperand("""
+        main {
+            word @shared err
+            word @shared speed
+            sub start() {
+                err = 80
+                speed = 1
+                speed = min(err / 8, speed)
+            }
+        }""", "min")
+    }
+
+    test("min/max keeps comparison when target aliases the first operand") {
+        assertNotReducedToOneOperand("""
+        main {
+            word @shared err
+            word @shared speed
+            sub start() {
+                err = 80
+                speed = 1
+                speed = min(speed, err / 8)
+            }
+        }""", "min")
+    }
+
+    test("max keeps comparison when target aliases the second operand") {
+        assertNotReducedToOneOperand("""
+        main {
+            word @shared err
+            word @shared speed
+            sub start() {
+                err = -80
+                speed = -1
+                speed = max(err / 8, speed)
+            }
+        }""", "max")
+    }
+
+    test("max keeps comparison when target aliases the first operand") {
+        assertNotReducedToOneOperand("""
+        main {
+            word @shared err
+            word @shared speed
+            sub start() {
+                err = -80
+                speed = -1
+                speed = max(speed, err / 8)
+            }
+        }""", "max")
+    }
+
     test("pointer indexing inside other expression ok") {
         val src="""
             main{

compiler/test/ast/TestAstChecks.kt

@@ -1,5 +1,6 @@
 package prog8tests.ast
 
+import io.kotest.assertions.withClue
 import io.kotest.core.spec.style.FunSpec
 import io.kotest.engine.spec.tempdir
 import io.kotest.matchers.nulls.shouldNotBeNull
@@ -812,4 +813,34 @@ main {
         compileText(VMTarget(), false, src, outputDir, errors, false) shouldNotBe null
         errors.errors.size shouldBe 0
     }
+
+    test("boolean expressions mixing not with or/and compile") {
+        // the 'not' simplification and the De Morgan rewrite of the same subtree used to both queue an
+        // ast modification, which corrupted the parent links of the nodes they shared (compiler crash)
+        val conditions = listOf(
+            "a == b or not (a >= b)",
+            "not (a >= b) or not (c >= b)",
+            "not (a >= b) or c == b",
+            "not (a == 0) and b == 0",
+            "a == b and not (a >= b)",
+            "(a == b) == 0 or not (c >= d)"
+        )
+        conditions.forEach { condition ->
+            val src = """
+                main {
+                    sub start() {
+                        uword a,b,c,d
+                        if $condition {
+                            a++
+                        }
+                    }
+                }
+            """.trimIndent()
+            val errors = ErrorReporterForTests()
+            compileText(Cx16Target(), true, src, outputDir, errors, false) shouldNotBe null
+            withClue(condition) {
+                errors.errors.size shouldBe 0
+            }
+        }
+    }
 })

compiler/test/codegeneration/TestExecution6502.kt

@@ -594,4 +594,69 @@ class TestExecution6502 : FunSpec({
         machine.assertMemory(0x4007, 9999 and 0xff)
         machine.assertMemory(0x4008, (9999 shr 8) and 0xff)
     }
+
+    test("lsb/msb/lsw/msw result delivered to a cx16 virtual register parameter") {
+        val src = $$"""
+            %option no_sysinit
+            %launcher none
+            %address $1000
+
+            main {
+                &ubyte poweroff = $f203
+
+                ; each routine copies the value it received in the @R0 virtual register to a probe address
+                asmsub recordLsb(ubyte c @R0) {
+                    %asm {{
+                        lda  cx16.r0L
+                        sta  $3000
+                        rts
+                    }}
+                }
+                asmsub recordMsb(ubyte c @R0) {
+                    %asm {{
+                        lda  cx16.r0L
+                        sta  $3001
+                        rts
+                    }}
+                }
+                asmsub recordLsw(uword c @R0) {
+                    %asm {{
+                        lda  cx16.r0
+                        sta  $3002
+                        lda  cx16.r0+1
+                        sta  $3003
+                        rts
+                    }}
+                }
+                asmsub recordMsw(uword c @R0) {
+                    %asm {{
+                        lda  cx16.r0
+                        sta  $3004
+                        lda  cx16.r0+1
+                        sta  $3005
+                        rts
+                    }}
+                }
+
+                sub start() {
+                    uword @shared l
+                    l = $1234
+                    recordLsb(lsb(l))
+                    recordMsb(msb(l))
+                    recordLsw(lsw(l))
+                    recordMsw(msw(l))
+                    poweroff = 1
+                }
+            }
+        """.trimIndent()
+
+        val compileResult = compileText(Cx16Target(), false, src, outputDir)
+        val machine = compileResult!!.simulate()
+        machine.assertMemory(0x3000, 0x34)      // lsb($1234)
+        machine.assertMemory(0x3001, 0x12)      // msb($1234)
+        machine.assertMemory(0x3002, 0x34)      // lsw($1234) low
+        machine.assertMemory(0x3003, 0x12)      // lsw($1234) high
+        machine.assertMemory(0x3004, 0x00)      // msw($1234) low
+        machine.assertMemory(0x3005, 0x00)      // msw($1234) high
+    }
 })

compiler/test/vm/TestCompilerVirtual.kt

@@ -66,7 +66,7 @@ main {
     }
 
     test("taking address of split arrays works") {
-        val src="""
+        val src = """
 main {
     sub start() {
         cx16.r0L=0
@@ -486,6 +486,104 @@ main {
         cmpiInstr shouldBe null
     }
 
+    test("short-circuit boolean and/or always emits explicit CMPI before the branch (strict status-bits contract)") {
+        val src = """
+main {
+    uword aa
+    uword bb
+    sub start() {
+        aa = 5
+        bb = 9
+        bool @shared res = aa == bb or aa < bb
+        bool @shared res2 = aa == bb and aa < bb
+    }
+}"""
+        val result = compileText(VMTarget(), optimize=false, src, outputDir, writeAssembly=true)!!
+        val virtfile = result.compilationOptions.outputDir.resolve(result.compilerAst.name + ".p8ir")
+        val irProgram = IRFileReader().read(virtfile)
+        val start = irProgram.blocks[0].children[0] as IRSubroutine
+        val instructions = start.chunks.flatMap { c->c.instructions }
+
+        // Both short-circuit operators must test the left operand with an explicit CMPI #0 before
+        // branching; without it the branch reads the status bits of whatever instruction ran before,
+        // which gave wrong results. A BST that directly follows a CMP is the comparison itself
+        // materialising its result, that one is fine.
+        // See CpuType.statusBitsOnMultiByteOps for the rationale.
+        val branches = instructions.withIndex().filter { it.value.opcode == Opcode.BSTEQ || it.value.opcode == Opcode.BSTNE }
+        withClue("branch instructions found") {
+            branches.size shouldBeGreaterThan 0
+        }
+        branches.forEach { (idx, branch) ->
+            val before = instructions[idx - 1]
+            withClue("instruction before $branch") {
+                val testedByCmp = before.opcode == Opcode.CMP
+                val testedByCmpi = before.opcode == Opcode.CMPI && before.immediate == 0
+                testedByCmp || testedByCmpi shouldBe true
+            }
+        }
+    }
+
+    test("short-circuit boolean and/or compute the correct value in the vm") {
+        val src = """
+main {
+    uword a
+    uword b
+    bool p
+    sub start() {
+        ; every case adds its own bit to cx16.r0 when the expression is true,
+        ; so a wrong result shows up as a difference in the bit pattern
+        cx16.r0 = 0
+        a = 5
+        b = 5
+        bool t = a == b or a < b           ; true
+        if t
+            cx16.r0 += 1
+        a = 5
+        b = 9
+        t = a == b or a < b                 ; true
+        if t
+            cx16.r0 += 2
+        a = 9
+        b = 5
+        t = a == b or a < b                 ; false
+        if t
+            cx16.r0 += 4
+        a = 1
+        b = 5
+        t = a == b and a < b                ; false
+        if t
+            cx16.r0 += 8
+        a = 9
+        b = 5
+        t = a == b and a < b                ; false
+        if t
+            cx16.r0 += 16
+        ; a bool variable as left operand: no comparison runs before the branch
+        p = false
+        a = 5
+        b = 9
+        t = p or a < b                      ; true
+        if t
+            cx16.r0 += 32
+        p = true
+        t = p and a < b                     ; true
+        if t
+            cx16.r0 += 64
+        a = 9
+        b = 5
+        t = p and a < b                     ; false
+        if t
+            cx16.r0 += 128
+    }
+}"""
+        val result = compileText(VMTarget(), optimize=true, src, outputDir, writeAssembly=true)!!
+        val virtfile = result.compilationOptions.outputDir.resolve(result.compilerAst.name + ".p8ir")
+        VmRunner().runAndTestProgram(virtfile.readText()) { vm ->
+            // bits 1, 2, 32 and 64 must be set: 1 + 2 + 32 + 64
+            vm.memory.getUW(0xff02u) shouldBe 99u
+        }
+    }
+
     test("repeat counts (const)") {
         val src="""
 main {
@@ -647,6 +745,19 @@ main {
         instructions[10].type shouldBe IRDataType.WORD
     }
 
+    test("word pop comparison uses the word result type") {
+        val src=$$"""
+main {
+    sub start() {
+        ubyte a
+        if popw() != $abcd {
+            a++
+        }
+    }
+}"""
+        compileText(VMTarget(), false, src, outputDir, writeAssembly = true) shouldNotBe null
+    }
+
     test("typed address-of a const pointer with non-const array indexing") {
         val src= """
 main {

compilerAst/src/prog8/ast/walk/AstWalker.kt

@@ -4,6 +4,8 @@ import prog8.ast.*
 import prog8.ast.expressions.*
 import prog8.ast.statements.*
 import prog8.code.core.CommutativeOperators
+import java.util.Collections
+import java.util.IdentityHashMap
 
 
 /**
@@ -26,6 +28,15 @@ open class AstModification {
     /** Nodes that will be affected when [perform] is called. */
     open val affectedNodes: List<Node> = emptyList()
 
+    /**
+     * The node in which a child is modified by [perform], if there is one.
+     * Modifications whose target parent node (or a node higher up in its parent chain) is no
+     * longer part of the tree are skipped by [AstWalker.applyModifications]: they would only
+     * change a subtree that has been detached by another modification, and re-linking the
+     * parents of nodes that are still in use elsewhere corrupts the tree.
+     */
+    open val targetParent: Node? = null
+
     /** Apply the modification to the AST. */
     open fun perform() {}
 }
@@ -41,6 +52,7 @@ open class AstRemove(val node: Node, val parent: IStatementContainer) : AstModif
     }
 
     override val affectedNodes = listOf(node)
+    override val targetParent: Node get() = parent as Node
 
     override fun perform() {
         if (!parent.statements.remove(node)) {
@@ -60,6 +72,7 @@ open class AstSetExpression(
     private val parent: Node
 ) : AstModification() {
     override val affectedNodes = listOf(newExpr)
+    override val targetParent: Node get() = parent
 
     override fun perform() {
         setter(newExpr)
@@ -77,6 +90,7 @@ open class AstInsert private constructor(
     private val position: InsertPosition
 ) : AstModification() {
     override val affectedNodes = listOf(stmt)
+    override val targetParent: Node get() = parent as Node
 
     override fun perform() {
         val idx = when (position) {
@@ -113,6 +127,7 @@ open class AstReplaceNode(
     private val parent: Node
 ) : AstModification() {
     override val affectedNodes = listOf(node, replacement)
+    override val targetParent: Node get() = parent
 
     override fun perform() {
         parent.replaceChildNode(node, replacement)
@@ -273,6 +288,12 @@ abstract class AstWalker {
     protected val modifications = mutableListOf<Triple<AstModification, Node, Node>>()
     protected val affectedModules = mutableSetOf<Module>()
 
+    /**
+     * The nodes that have been replaced or removed by a modification that was applied already,
+     * and thus are no longer part of the tree.
+     */
+    private val detachedNodes = Collections.newSetFromMap(IdentityHashMap<Node, Boolean>())
+
     private fun track(mods: Iterable<AstModification>, node: Node, parent: Node) {
         if (mods.any()) {
             try {
@@ -293,6 +314,11 @@ abstract class AstWalker {
         }
     }
 
+    /**
+     * Applies all queued modifications, skipping modifications that would operate on a node
+     * or subtree that is no longer part of the tree (because an earlier modification of the
+     * same batch detached it). Returns the number of modifications that were performed.
+     */
     open fun applyModifications(): Int {
         // Build conflict map for debugging - detect nodes affected by multiple modifications
         val nodeToModifications = mutableMapOf<Node, MutableList<AstModification>>()
@@ -315,14 +341,55 @@ abstract class AstWalker {
             }
         }
 
+        detachedNodes.clear()
+        var amount = 0
         modifications.forEach {
-            it.first.perform()
+            val mod = it.first
+            if (mod.operatesInDetachedSubtree()) {
+                // another modification of this batch already detached the subtree that this
+                // modification would operate in, so applying it has no effect on the tree -
+                // and if it reuses nodes that are still in use elsewhere, re-linking their
+                // parents corrupts the tree. Skip it.
+                return@forEach
+            }
+            mod.perform()
+            when (mod) {
+                is AstReplaceNode -> detachedNodes += mod.node
+                is AstRemove -> detachedNodes += mod.node
+                else -> { }
+            }
+            amount++
         }
-        val amount = modifications.size
         modifications.clear()
         return amount
     }
 
+    /** True if [node] is itself detached, or lies inside a subtree that was detached by a modification that was applied earlier in this batch. */
+    private fun isDetached(node: Node?): Boolean {
+        if (node == null || detachedNodes.isEmpty()) return false
+        if (node in detachedNodes) return true
+        var candidate = node.parent
+        while (candidate !is ParentSentinel) {
+            if (candidate in detachedNodes) return true
+            candidate = candidate.parent
+        }
+        return false
+    }
+
+    /**
+     * True if this modification would operate on a node or subtree that is no longer part of
+     * the tree: performing it would have no effect on the live tree, or could corrupt the
+     * parent links of nodes that are still in use by a replacement.
+     */
+    private fun AstModification.operatesInDetachedSubtree(): Boolean {
+        if (isDetached(targetParent)) return true
+        return when (this) {
+            is AstReplaceNode -> isDetached(node)
+            is AstRemove -> isDetached(node)
+            else -> false
+        }
+    }
+
     fun linkAffectedParents(namespace: Node) {
         affectedModules.forEach { it.linkParents(namespace) }
         affectedModules.clear()

examples/test.p8

@@ -1,14 +1,12 @@
-main {
-    const uword STACK_SIZE = 10
-    struct Stack {
-        uword[STACK_SIZE] stack1
-        uword[20] stack2
-        uword top
-    }
+%import textio
+%zeropage basicsafe
 
+main {
     sub start() {
-        ^^Stack st = []
-        st.stack1[5] = 9999
-        st.stack2[5] = 8888
+
+        for cx16.r0L in 5 downto 0 {
+            txt.print_ub(cx16.r0L)
+            txt.spc()
+        }
     }
 }

fix-deref.md

@@ -0,0 +1,171 @@
+# Pointer Dereference Codegen Analysis
+
+## Bug
+
+In the old 6502 codegen, a bare typed-pointer dereference such as:
+
+```prog8
+^^pointer temp = &table
+while temp^^ != $0000 {
+    temp++
+}
+```
+
+corrupted `temp` itself. The generated code loaded the value pointed to by `temp` and wrote it back into `temp`, so the next iteration read from the wrong address.
+
+## Root Cause
+
+`PointerAssignmentsGen.deref()` in `codeGenCpu6502` does not return a uniform abstraction. Depending on the length of the pointer-deref chain, it returns one of three different things:
+
+| Chain length | Example | What `deref()` returns |
+|--------------|---------|------------------------|
+| 0 | `temp^^` | the pointer variable itself |
+| 1 | `foo.bar^^` | address of the final pointer field |
+| >1 | `foo.bar.baz^^` | address of the final value (the pointer stored in the final field is loaded into `P8ZP_SCRATCH_PTR`, offset 0) |
+
+`assignPointerDerefExpression()` then had to know which case it received. The original code always performed an extra word load and wrote it back into the returned pointer variable:
+
+```kotlin
+asmgen.loadIndirectWordAY(zpPtrVar, offset1)
+asmgen.out("  sta  $zpPtrVar |  sty  $zpPtrVar+1")
+```
+
+This is safe when `zpPtrVar` is the temporary scratch pointer `P8ZP_SCRATCH_PTR` (field-access cases), but wrong when `zpPtrVar` is the user's own pointer variable (bare `temp^^`), because it overwrites the variable.
+
+## Applied Fix
+
+Changed the extra dereference in `assignPointerDerefExpression()` to run only for non-empty chains:
+
+```kotlin
+if(value.derefLast && value.chain.isNotEmpty()) {
+    asmgen.loadIndirectWordAY(zpPtrVar, offset1)
+    asmgen.out("  sta  $zpPtrVar |  sty  $zpPtrVar+1")
+    offset = 0.toUByte()
+} else {
+    offset = offset1
+}
+```
+
+- Bare variable derefs (`temp^^`) now read directly from the variable without corrupting it.
+- Struct-field pointer derefs (`foo.bar^^`, `foo.bar.baz^^`) still get the extra dereference they need, because `deref()` uses `P8ZP_SCRATCH_PTR` for those cases.
+
+Tests added:
+
+- `double pointer dereference in while loop (code gen fix)` — covers the bare-variable case.
+- `pointer-to-pointer struct field deref still double-derefs on 6502` — preserves the existing double-deref behavior for struct fields.
+
+## Comparison with IR-Based Codegens
+
+The IR codegens (`codeGenIntermediate`, used by virtual, `new6502`, and m68k) handle this more cleanly. In `ExpressionGen.kt`:
+
+1. `translate(startpointer)` evaluates the base pointer into a register.
+2. `traverseRestOfDerefChainToCalculateFinalAddress()` walks the chain and always produces a register that points at the final value (or a field offset when there is no final dereference).
+3. A single `LOADI` reads from `(pointerReg, offset)`.
+
+The empty-chain case falls out naturally: the traverse function does nothing, so the load reads directly from the startpointer register. The caller never has to decode different abstraction levels based on chain length.
+
+## Remaining Technical Debt
+
+The precise inconsistency is narrower than the chain-length table suggests. For chains of length 1 and >1, `deref()` already returns the same abstraction: a pointer to the final value plus a Y offset. Only the empty-chain case differs: `deref()` may return the user's own zeropage pointer variable (when `isZpVar()`), and by construction any caller write-back into the returned variable is forbidden there, whereas in all other cases the returned variable is `P8ZP_SCRATCH_PTR`, which callers may safely write back into.
+
+The current fix only prevents the write-back in `assignPointerDerefExpression()`. A proper cleanup would make `deref()` always return `P8ZP_SCRATCH_PTR` (copying the user's pointer there when the chain is empty), or alternatively make the contract explicit with a "may clobber the returned pointer" flag that callers must obey. Either form restores one uniform abstraction for all chain lengths and lets `assignPointerDerefExpression()` treat every case identically, matching the IR codegen structure in `ExpressionGen.kt`. The trade-off of always copying to the scratch pointer is one extra 2-byte copy for the common bare-variable case.
+
+## Verified Semantic Fact (empirical)
+
+Test program on 2026-10-19 (both targets compiled with the current compiler build):
+
+```prog8
+struct Inner { ^^uword p }
+struct Outer { ^^Inner inner }
+^Outer o; ^Inner ip
+o.inner = ip ; ip.p = $1000 ; pokew($1000, $aaaa) ; pokew($aaaa, $1234)
+result = o.inner.p^^
+```
+
+- `virtual` target: prints `aaaa`.
+- `c64` target: generated assembly computes `AY = word at (scratch)` (= value of field `p`, `$1000`) followed by `jsr prog8_lib.func_peekw`, so `result = *(p content)` = `aaaa` as well.
+
+So both backends agree on the invariant: **for `derefLast` chains the value is `*(pointer stored in the final field)`; each pointer in the chain is followed exactly once, plus one read of the pointed-to word.** This validates the fix's premise (double deref for `foo.bar^^`, via implicit follow of `o` plus follow of the stored pointer). Note also that `CodeDesugarer` rewrites many scalar-field deref chains into `peekw/peekl` calls before codegen (see `CodeDesugarer.kt:1569-1622`), so `deref()` only ever receives the shapes that survive desugaring; the exact inventory of surviving shapes is part of phase 0 below.
+
+## Proposed Plan: normalize `deref()` to a single contract
+
+### Target contract (mirrors the IR codegen)
+
+`deref()` always returns `(zpVar, offset)` such that **one indirect access through `zpVar` at byte `offset` reaches the final thing**:
+
+- `derefLast=true`: `(zpVar),offset` is the address of the value itself. One word read yields the value; one memory write with `(zpVar),offset` stores it.
+- `derefLast=false`: `(zpVar),offset` is the location of the field content (typically a pointer field); reads and writes go directly through it.
+- `addOffsetToPointer=true` and `loadPointerFieldValue=true`: unchanged; this mode is already "materialize the effective address" and every current subbranch already consumes all follows it needs (`PointerAssignmentsGen.kt:279-304`, follows for `derefLast`).
+
+Key point: this is precisely the IR codegen's contract (`traverseRestOfDerefChainToCalculateFinalAddress` returns a reg+offset pair) so that the same single `LOADI`-equivalent works everywhere.
+
+### Current conformance gaps (all in `deref()`, only for `derefLast`)
+
+| shape | follows inside `deref()` | conforms |
+|-------|--------------------------|----------|
+| chain 0 (`temp^^`)      | 0 (`deref()` returns the variable itself) | yes (`(temp),off` already reaches the value) |
+| chain 1, `addOffsetToPointer=true` | final field followed (`deref:282-284`) | yes |
+| chain 1, `addOffsetToPointer=false` | final field NOT followed (`deref:286-304`); caller must follow | **no** |
+| chain >1                | every field followed, including the final one (`deref:312-328`) | yes |
+| chain 1/`>1`, `derefLast=false` | field content location returned with offset | yes |
+
+So chain>1 vs chain<=1 asymmetry is the *only* structural leak: the fixed `assignPointerDerefExpression()` has to decide "was the final follow consumed?" by testing `chain.isNotEmpty()`.
+
+### Phase 1 - move the final follow into deref() (mechanical)
+
+In the `chain.size==1 && !addOffsetToPointer && derefLast` subbranches (`PointerAssignmentsGen.kt:286-304`, both the `isZpVar` and the copy-to-scratch path), call `updateScratchPointer()` right after `addFieldOffsetToScratchPointer()` and return `(P8ZP_SCRATCH_PTR, 0)`. After phase 2 removes the now-duplicated caller-side follow, the follow is neither duplicated nor dropped.
+
+Phase 1 is NOT size-neutral or behavior-neutral: only `assignPointerDerefExpression()` has a matching follow to delete. Every other caller of `deref()` with default args receives a followed pointer plus one extra `updateScratchPointer` (~8 bytes) for chain==1 `derefLast`, with nothing to remove on their side. The affected call sites are the assign* helpers (`PointerAssignmentsGen.kt:37-230`, writes and inplace negate/invert), `inplaceModificationPointerField` (`AugmentableAssignmentAsmGen.kt:134`), the const-index pointer-array read (`AssignmentAsmGen.kt:262`), the address-of read paths (`AssignmentAsmGen.kt:563,749`, `PointerAssignmentsGen.kt:945`), and the `AugmentableAssignmentAsmGen` RMW dispatch. For write targets (`foo.bar^^ = v`) this also changes semantics: today those helpers store into the field location (`&foo.bar`), after phase 1 they store through the followed pointer (`*(foo.bar)`), which matches the IR backend. This is an intended behavior change, not a relocation; size deltas of a few bytes UP per affected site are expected and must be attributed per the documented rule.
+
+### Phase 2 - delete the caller's private final follow (mechanical, small)
+
+Only one call site has a compensating follow to delete: `assignPointerDerefExpression()` (`PointerAssignmentsGen.kt:397-408`) is the only place in the codebase with the `loadIndirectWordAY(zpPtrVar, offset1)` + `sta $zpPtrVar | sty $zpPtrVar+1` write-back pattern (verified by grep on 2026-10-19). Delete the entire `if(value.derefLast && value.chain.isNotEmpty())` block there; the bare-read path that was the "fix" stays as the only path. Do not hunt for further follows to remove: every other `loadIndirectWordAY(zpPtrVar, 0u)` site is either behind `addOffsetToPointer=true` (a branch phase 1 does not touch) or is an RMW load of the current value being multiplied, divided, or reduced (`PointerAssignmentsGen.kt:1941-2104`), which must be kept.
+
+Also required: a comment update to `derefUsesScratchPointer()` to match the new contract; no behavioral change there (which paths use `P8ZP_SCRATCH_PTR` does not change - we only added one follow instruction inside already-scratch paths).
+
+### Phase 0 (before phase 1) - instrument the surviving shapes
+
+`CodeDesugarer` already converts a large class of scalar-field deref chains into `peek/peekw/peekl` calls, so `deref()` is only exercised for a subset of shapes. Build a dev-only instrumented `deref()` that logs `(chain.size, derefLast, addOffsetToPointer, loadPointerFieldValue, isZpVar)` tuples across `prog8c` on the whole `examples/` tree and the compiler's own test corpus, plus all new tests. Any tuple that does not match the conformance table above is discovered before it can break a caller-cleanup edit.
+
+### Phase 3 - verification
+
+- Extend `TestPointers.kt` with tests for the branch phase 1 actually changes - chain==1 `derefLast`: a read (`foo.bar^^`, asserting the double deref result, as a `simulate()`-based test) and a write (`foo.bar^^ = v`, asserting the store lands at `*(foo.bar)`, not in the field). The existing test `pointer-to-pointer struct field deref still double-derefs on 6502` is the regression anchor for the read side and must stay green.
+- Note what does NOT cover the change: the discriminating `$aaaa`/`$1234` chain>1 read program from the section above is desugared into `peekw(...)` before the 6502 codegen (detriment #1), so a `simulate()` test of it exercises the desugarer+peekw path and passes regardless of phase 1/2 correctness. A chain>1 write (`x.inner.p^^ = v`) exercises the chain>1 branch, which phase 1 does not modify. Both are fine as additional coverage but cannot substitute for the chain==1 tests.
+- `prog8tests.compiler.TestPointers` green; then `TestCompilerOnExamples` expected sizes, following the documented rules: diff the generated `.asm` of shifted examples before updating each expected size (phase 1 shifts sizes UP slightly at chain==1-`derefLast` sites - see phase 1; phase 2 shifts sizes down slightly).
+- Final guards: `-noopt` baseline runs are already covered by existing tests; no new failure modes expected because no optimizer participates in this change.
+
+### Alternative rejected: explicit three-way contract
+
+A cheaper fix would change the return type to a small data class `(zpVar, offset, finalFollowConsumed: Boolean)` so each caller branches on data instead of guessing by chain length. Rejected as the main plan because it makes the leak permanent: every current and future caller must still branch, and the internal-only inconsistency survives. It remains the right fallback if phase-2 caller cleanup turns out to be riskier than expected.
+
+### Potential detriments: cases the implementer must verify
+
+The normalization itself touches only the chain==1-`derefLast`, `addOffsetToPointer=false` subbranches of `deref()` (chain 0, chain>1, and all `addOffsetToPointer=true` paths are untouched), but the following cases can be silently damaged by an over-eager phase-2 cleanup. Each lists the today-evidence and what to verify before and after the edit (compile with and without `-noopt`, diff `.asm`):
+
+1. **Chain>1 value reads have already been desugared away - do not delete their "follows".**
+   Verified on 2026-10-19: `result = o.inner.p^^` reaches the codegen as `peekw(<derefLast=false chain arg>)` (desugarer rewrite); the emitted asm shows exactly one `updateScratchPointer` (the chain>1 `derefLast` branch is bypassed) plus `jsr prog8_lib.func_peekw`. The `loadIndirectWordAY(zpPtrVar, 0u)`-shaped site that loads the field pointer for the peek argument is a **field read**, not a leftover follow. Phase 2's "delete the self-follow" rule applies **only** to the `loadIndirectWordAY(...)` + `sta $zpPtrVar | sty $zpPtrVar+1` write-back pattern behind `derefLast=true`. Deleting a plain derefLast=false value load corrupts the peek argument.
+
+2. **Address-of-derefLast shapes change meaning under phase 1.**
+   `PtAddressOf` on a chain (`AssignmentAsmGen.kt:740-764`, `PointerAssignmentsGen.readByteByAddressOfDereference:943-953`): for chain 0, today's behavior (copy the pointer variable's content = the value address) is correct and unchanged. For chain 1 with `derefLast`, today's callers consume the returned var as "location holding the pointer to be dereferenced"; after phase 1 `deref()` returns the followed pointer instead, shifting such reads/writes by one indirection level. Verify whether any `&(chain-1-derefLast)` shape survives `CodeDesugarer` at all; if yes, align to the IR backend's semantics (address-of-value = the followed pointer) and add `simulate()` regression tests for it. (The swap paths in `AsmGen.kt` are NOT affected: the non-optimized swap calls `deref(v, true)` with `addOffsetToPointer=true`, and the zeropage fast path at `AsmGen.kt:1550` bypasses `deref()` entirely.)
+
+3. **RMW / augmented-assignment paths must keep using the same scratch layout.**
+   `AugmentableAssignmentAsmGen.inplaceModificationPointerField` has a hard `require(zpPtrVar=="P8ZP_SCRATCH_PTR")` and relies on `derefUsesScratchPointer()` to decide between in-place-through-variable vs the preserve-address-on-stack scheme. Phase 1 must not flip any of those decisions (chain-0 and chain-1-`!derefLast` zp fast paths stay zp-var-based). Verify with before/after asm diffs on `temp^^ += 1`, `foo.bar^^ += 1`, and bare derefLast targets; any flip from user-var to scratch (or back) changes the clobber-preservation assumptions and needs paired edits.
+
+4. **`derefUsesScratchPointer()` re-derivation.**
+   No behavior change is expected (it keys on which paths use the scratch pointer, and phase 1 keeps exactly those paths scratch-bound), but grep callers for other places that branch on "deref returned a user variable for chain==1" (`isZpVar` checks inside callers) and update comments/conditions where the contract terminology changed.
+
+5. **Sizes.**
+   Phase 1 shifts sizes UP slightly (one extra `updateScratchPointer` per chain==1-`derefLast` default-arg call site; see phase 1), phase 2 shifts sizes down slightly (the single deleted follow in `assignPointerDerefExpression()`). `TestCompilerOnExamples` expected sizes may also shift if the desugarer/CPU-optimizer interacts with restructured patterns. Follow the documented rule: revert + re-diff the generated `.asm` for each shifted example before bumping an expected size.
+
+### Explicitly out of scope: existing frontend pointer-chain issues
+
+Several pointer-chain compilation problems exist independently of `deref()` and belong to the deferred `todo.rst` item ("really fixing the pointer dereferencing issues - cursed hybrid between IdentifierReference, PtrDereference and PtrIndexedDereference"). Verified on 2026-10-19 on the current build: `while o.inner.p^^ != $aaaa { ... }` crashes the compiler with `FatalAstException: binexpr with '.' operator should have been converted into PtrDereference or something else` (AstChecker.kt:2603) - distinct frontend bug, ditto for `-noopt`. The `deref()` normalization work must NOT attempt to fix these.
+
+Practical rules for the implementer:
+
+- Phase 0's shape inventory must be recorded *per usage context* (assignment RHS, if/while condition, call argument, augmented assignment, swap, address-of), because `CodeDesugarer` produces contextually different shapes and some contexts don't compile at all today.
+- Frontend crashes found during phase 0 are recorded as known-broken input values for the inventory, and are not fixed here.
+- Regression policy: if a program fails after the refactor, first reproduce on the pre-refactor build (revert, rebuild). Only failures that appear with the refactor but not without it are in scope for this change; everything else is a separate bug report against the deferred pointer-hybrid item.
+- The alternative "always return `P8ZP_SCRATCH_PTR` including chain 0" variant listed under Remaining Technical Debt IS detrimental (extra 2-byte copy + follow on every bare-deref read, the hottest shape - e.g. the while-loop scan that motivated the original fix); the follow-relocation plan above deliberately avoids that cost. If that variant is ever revisited, re-measure the bare-deref loops first.
+
+See `docs/source/todo.rst` for the tracking item.

gradle.properties

@@ -4,4 +4,4 @@ org.gradle.parallel=true
 org.gradle.daemon=true
 org.gradle.configuration-cache=false
 kotlin.code.style=official
-version=12.3.4-SNAPSHOT
+version=12.3.5-SNAPSHOT