diff --git a/AGENTS.md b/AGENTS.md index 3520ffb5a..5c32cf612 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,9 +38,11 @@ Source → parseMainModule() → processAst() → optimizeAst() → postprocessA ## DEBUGGING TIP: Use `-noopt` to isolate problems -When debugging compiler issues, **FIRST try compiling with the `-noopt` switch**: +When investigating a possible code generation problem (both IR and 6502), **FIRST try compiling the program with the `-noopt` switch** to disable most of the compiler optimizations. - **Problem gone with `-noopt`**: Issue is in **optimization phases** (`optimizeAst()`, `UnusedCodeRemover`, `Inliner`, etc.) -- **Problem persists with `-noopt`**: Issue is in **parsing, semantic analysis, symboltable, or code generation** +- **Problem persists with `-noopt`**: Issue is in **parsing, semantic analysis, symboltable, or the regular code generation path**. + +This way you can determine if the problem is caused by a faulty optimization step, or just occurs in the regular code generation path. ## DEBUGGING TIP: Use `-compareir` to see what changed When investigating optimization-related issues or tracking regressions: diff --git a/codeCore/src/prog8/code/core/BuiltinFunctions.kt b/codeCore/src/prog8/code/core/BuiltinFunctions.kt index def96e0f9..ae2f22dea 100644 --- a/codeCore/src/prog8/code/core/BuiltinFunctions.kt +++ b/codeCore/src/prog8/code/core/BuiltinFunctions.kt @@ -58,6 +58,7 @@ class FSignature(val pure: Boolean, // does it have side effects? } } } else { + // note: this path is currently never hit because the builtin functions with multiple return values have been handled in specialized optimized codegens already earlier. TODO("multiple return types from builtin function") } @@ -193,4 +194,4 @@ val InplaceModifyingBuiltinFunctions = setOf( val SimpleBuiltinFunctions = setOf( "msb", "lsb", "msw", "lsw", "mkword", "mklong", "mklong2", - "set_carry", "set_irqd", "clear_carry", "clear_irqd") \ No newline at end of file + "set_carry", "set_irqd", "clear_carry", "clear_irqd") diff --git a/codeGenIntermediate/src/prog8/codegen/intermediate/IRPeepholeOptimizer.kt b/codeGenIntermediate/src/prog8/codegen/intermediate/IRPeepholeOptimizer.kt index d682e9083..d4b8931d0 100644 --- a/codeGenIntermediate/src/prog8/codegen/intermediate/IRPeepholeOptimizer.kt +++ b/codeGenIntermediate/src/prog8/codegen/intermediate/IRPeepholeOptimizer.kt @@ -616,6 +616,12 @@ jump p8_label_gen_2 val deadStores = mutableSetOf() indexedInstructions.forEach { (idx, ins) -> + if (ins.opcode in OpcodesWithSideEffects) { + // Instruction has side effects, must never be removed as dead store, + // AND it also counts as a "read" of all current pending registers to avoid them being removed + pendingWrites.clear() + return@forEach + } val formats = instructionFormats.getValue(ins.opcode) val format = formats[ins.type] ?: formats[null] diff --git a/compiler/test/vm/TestDivmodFix.kt b/compiler/test/vm/TestDivmodFix.kt new file mode 100644 index 000000000..831761f14 --- /dev/null +++ b/compiler/test/vm/TestDivmodFix.kt @@ -0,0 +1,96 @@ +package prog8tests.vm + +import io.kotest.assertions.fail +import io.kotest.core.spec.style.FunSpec +import io.kotest.engine.spec.tempdir +import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe +import prog8.code.core.IErrorReporter +import prog8.code.core.Position +import prog8.code.target.VMTarget +import prog8.intermediate.IRFileReader +import prog8.vm.VmRunner +import prog8.vm.VmVariableAllocator +import prog8tests.helpers.ErrorReporterForTests +import prog8tests.helpers.compileText +import kotlin.io.path.readText + +class TestDivmodFix : FunSpec({ + test("divmod does not crash when results are unused (optimization issue)") { + val outputDir = tempdir().toPath() + val src = """ + %zeropage basicsafe + %option no_sysinit + main { + sub start() { + ubyte num = 230 + ubyte div = 13 + ubyte d, r = divmod(num, div) + } + } + """.trimIndent() + + val innerErrors = ErrorReporterForTests(throwExceptionAtReportIfErrors = false, keepMessagesAfterReporting = true) + val errors = object : IErrorReporter by innerErrors { + override fun printSingleError(errormessage: String) { + innerErrors.errors.add(errormessage) + } + } + val result = try { + compileText(VMTarget(), true, src, outputDir, writeAssembly = true, errors = errors) + } catch (e: Exception) { + fail("Compilation threw exception (unused): $e\n" + e.stackTraceToString() + "\nErrors:\n" + innerErrors.errors.joinToString("\n")) + } + if (result == null || innerErrors.errors.isNotEmpty()) { + fail("Compilation failed (unused):\n" + innerErrors.errors.joinToString("\n")) + } + val virtfile = result.compilationOptions.outputDir.resolve(result.compilerAst.name + ".p8ir") + VmRunner().runProgram(virtfile.readText(), false) + } + + test("divmod results are correct with optimizations") { + val outputDir = tempdir().toPath() + val src = """ + %zeropage basicsafe + %option no_sysinit + main { + ubyte d_res + ubyte r_res + sub start() { + d_res = 0 + r_res = 0 + ubyte num = 230 + ubyte div = 13 + ubyte d, r = divmod(num, div) + d_res = d + r_res = r + } + } + """.trimIndent() + + val innerErrors = ErrorReporterForTests(throwExceptionAtReportIfErrors = false, keepMessagesAfterReporting = true) + val errors = object : IErrorReporter by innerErrors { + override fun printSingleError(errormessage: String) { + innerErrors.errors.add(errormessage) + } + } + val result = try { + compileText(VMTarget(), true, src, outputDir, writeAssembly = true, errors = errors) + } catch (e: Exception) { + fail("Compilation threw exception (correct results): $e\n" + e.stackTraceToString() + "\nErrors:\n" + innerErrors.errors.joinToString("\n")) + } + if (result == null || innerErrors.errors.isNotEmpty()) { + fail("Compilation failed (correct results):\n" + innerErrors.errors.joinToString("\n")) + } + + val virtfile = result.compilationOptions.outputDir.resolve(result.compilerAst.name + ".p8ir") + val irSource = virtfile.readText() + val irProgram = IRFileReader().read(irSource) + val allocations = VmVariableAllocator(irProgram.st, irProgram.encoding, irProgram.options.compTarget).allocations + + VmRunner().runAndTestProgram(irSource) { vm -> + vm.memory.getUB(allocations["main.d_res"]!!.toUInt()) shouldBe 17u + vm.memory.getUB(allocations["main.r_res"]!!.toUInt()) shouldBe 9u + } + } +}) diff --git a/examples/test.p8 b/examples/test.p8 index 382b1723e..79d5a7b9a 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,10 +1,28 @@ +%import textio +%zeropage basicsafe + main { + sub start() { - ubyte a=10 - cx16.r0++ - a = 20 - cx16.r0 = a - a = 30 - cx16.r1 = a + long @shared lv = $12345678 + + ubyte l,m,h = lmh(lv) + + txt.print_ubhex(h, true) + txt.spc() + txt.print_ubhex(m, false) + txt.spc() + txt.print_ubhex(l, false) + txt.nl() + + + ubyte @shared num = 230 + ubyte @shared div = 13 + + ubyte d,r = divmod(num, div) + txt.print_ub(d) + txt.spc() + txt.print_ub(r) + txt.nl() } } diff --git a/intermediate/src/prog8/intermediate/IRInstructions.kt b/intermediate/src/prog8/intermediate/IRInstructions.kt index 5215f3148..f8e46aac1 100644 --- a/intermediate/src/prog8/intermediate/IRInstructions.kt +++ b/intermediate/src/prog8/intermediate/IRInstructions.kt @@ -598,6 +598,18 @@ val OpcodesThatLoad = setOf( val OpcodesThatSetStatusbits = OpcodesThatSetStatusbitsButNotCarry + OpcodesThatSetStatusbitsIncludingCarry +val OpcodesWithSideEffects = OpcodesThatBranch + setOf( + Opcode.PUSH, + Opcode.POP, + Opcode.PUSHST, + Opcode.POPST, + Opcode.DIVMOD, + Opcode.DIVMODR, + Opcode.SDIVMOD, + Opcode.SDIVMODR, + Opcode.BREAKPOINT +) + enum class IRDataType { BYTE,