diff --git a/codeGenIntermediate/src/prog8/codegen/intermediate/StackFrameLayout.kt b/codeGenIntermediate/src/prog8/codegen/intermediate/StackFrameLayout.kt index f30a33d8c..84cd65432 100644 --- a/codeGenIntermediate/src/prog8/codegen/intermediate/StackFrameLayout.kt +++ b/codeGenIntermediate/src/prog8/codegen/intermediate/StackFrameLayout.kt @@ -20,6 +20,8 @@ import prog8.intermediate.* * - it contains no inline assembly chunk, * - it makes no indirect calls and its address is never taken (subptr dispatch tables, interrupt * handler registration, ...): an unknown caller cannot push arguments into a frame, + * - it cannot reach a subroutine whose assembly code writes to A5: such a call would overwrite + * the frame pointer while this subroutine's frame slots are still live, * - all its parameters are frameable (a parameter that cannot move forces the whole * subroutine to keep the static convention, since the caller must then write static cells), * - every local it moves is plain data (byte/word/long/pointer scalar or numeric/bool array) @@ -80,11 +82,13 @@ class StackFrameLayout(private val program: IRProgram, private val errors: IErro val dispatchTargets = collectDispatchTargets(subLabels, addressTaken) val indirectCallers = allSubs.filter { sub -> containsIndirectCall(sub) }.map { it.label }.toSet() val reentrant = subsWithMultipleLiveActivations(callSites, indirectCallers, dispatchTargets) + val framePointerClobberers = framePointerClobberingSubs() + val reachesFramePointerClobber = subsReachingFramePointerClobber(framePointerClobberers) val framedSubs = mutableMapOf() val notFramedReasons = mutableMapOf() for (sub in allSubs) { - val outcome = frameableSubroutine(sub, referencedIn, addressTaken, asmTexts, callSites, dispatchTargets, indirectCallers, reentrant) + val outcome = frameableSubroutine(sub, referencedIn, addressTaken, asmTexts, callSites, dispatchTargets, indirectCallers, reentrant, reachesFramePointerClobber) when (outcome) { is Frameability.Frameable -> framedSubs[sub.label] = outcome.layout is Frameability.NotFrameable -> notFramedReasons[sub.label] = outcome.reason @@ -148,6 +152,125 @@ class StackFrameLayout(private val program: IRProgram, private val errors: IErro return found } + /** + * Labels of subroutines whose code writes to A5, the frame pointer register of a framed + * subroutine. Assembly subroutines are matched on their assembly text, ordinary subroutines on + * the text of their inline assembly chunks (a subroutine without assembly cannot write A5). + * + * Such a subroutine invalidates the frame of its caller: every frame-slot reference in the + * caller then uses a different base register, and the caller's `unlk a5` restores a frame + * pointer that was never saved. + */ + private fun framePointerClobberingSubs(): Set { + val result = mutableSetOf() + program.allAsmSubs().forEach { asmSub -> + if (writesFramePointerRegister(asmSub.asmChunk.assembly)) + result += asmSub.label + } + for (sub in program.allSubs()) { + sub.forEachChunk { chunk -> + if (chunk is IRInlineAsmChunk && writesFramePointerRegister(chunk.assembly)) + result += sub.label + } + } + return result + } + + /** + * Subroutine label -> label of the frame-pointer-clobbering subroutine it can reach, for every + * subroutine that can (directly or indirectly) call one. An indirect call may dispatch to any + * subroutine, so a subroutine containing one is assumed to reach all of them. + */ + private fun subsReachingFramePointerClobber(clobberers: Set): Map { + if (clobberers.isEmpty()) + return emptyMap() + val callTargets = mutableMapOf>() + for (sub in program.allSubs()) { + val targets = callTargets.getOrPut(sub.label) { mutableSetOf() } + sub.forEachChunk { chunk -> + chunk.instructions.forEach { instr -> + when (val ref = instr.callSite?.codeReference) { + is CodeReference.Label -> targets += ref.name + is CodeReference.Indirect -> targets += clobberers + else -> {} + } + } + } + } + val reaching = mutableMapOf() + var changed = true + while (changed) { + changed = false + for ((label, targets) in callTargets) { + if (label in reaching) + continue + val hit = targets.firstOrNull { it in clobberers || it in reaching } ?: continue + reaching[label] = if (hit in clobberers) hit else reaching.getValue(hit) + changed = true + } + } + return reaching + } + + /** + * True when the assembly text writes to A5 without protecting the caller's frame pointer, in a + * form this textual check recognises: as the destination operand of an instruction, as the frame + * register of a `link`, or anywhere in a `movem` register list (whose mask syntax is not parsed + * here). The usual `move.l a5,-(sp)` ... `move.l (sp)+,a5` prologue/epilogue pair is recognised + * as balanced and does not count; a write in between does. + * + * The scan is linear and ignores branches, so a subroutine that saves A5 on one path and + * overwrites it on another is reported conservatively as a clobber. Being over-cautious only + * costs the calling subroutine its stack frame (it keeps the static convention instead). + */ + private fun writesFramePointerRegister(asm: String): Boolean { + var saved = false + for (rawLine in asm.lines()) { + // drop comments and a leading label, so a labelled instruction is still examined + val line = rawLine.substringBefore(';') + .replace(Regex("^[.0-9A-Za-z_$]+:"), "") + .trim().replace(Regex("[ \t]+"), " ") + if (line.isEmpty() || line.startsWith('.')) // equates and directives + continue + val words = line.split(' ') + val mnemonic = words[0].lowercase() + val operands = words.drop(1).joinToString(",").split(',') + if (operands.any { it.isEmpty() }) + continue + if (mnemonic.startsWith("movem")) { + val pushes = operands.last().lowercase() == "-(sp)" + val mentionsA5 = operands.any { it.lowercase().contains("a5") } + if (pushes && mentionsA5) { + saved = true + continue + } + if (!pushes && operands.first().lowercase() == "(sp)+" && mentionsA5) { + saved = false + continue + } + if (mentionsA5 && !saved) + return true + continue + } + if (mnemonic == "link" && operands[0].lowercase().removePrefix("-") == "a5") + return true + val source = operands.first().lowercase() + val destination = operands.last().lowercase() + val writesA5 = destination == "a5" || destination == "-a5" + if (destination == "-(sp)" && source == "a5") { + saved = true + continue + } + if (source == "(sp)+" && writesA5 && operands.size == 2) { + saved = false + continue + } + if (writesA5 && !saved) + return true + } + return false + } + /** * Labels of subroutines that can have two live activations at the same time: they lie on a * cycle of direct calls (direct recursion or mutual recursion), or on a cycle that only closes @@ -256,10 +379,14 @@ class StackFrameLayout(private val program: IRProgram, private val errors: IErro callSites: Map>, dispatchTargets: Set, indirectCallers: Set, - reentrant: Set + reentrant: Set, + reachesFramePointerClobber: Map ): Frameability { val static = { reason: String -> Frameability.NotFrameable(reason) } if (sub.hasFrame) return static("it already has a stack frame") + reachesFramePointerClobber[sub.label]?.let { + return static("it calls '$it', whose assembly code overwrites the frame pointer register A5") + } var hasInlineAsm = false sub.forEachChunk { chunk -> if (chunk is IRInlineAsmChunk) hasInlineAsm = true } if (hasInlineAsm) return static("it contains inline assembly") diff --git a/compiler/res/prog8lib/shared_amiga_custom.p8 b/compiler/res/prog8lib/shared_amiga_custom.p8 index c0e9568d4..48c9faaf9 100644 --- a/compiler/res/prog8lib/shared_amiga_custom.p8 +++ b/compiler/res/prog8lib/shared_amiga_custom.p8 @@ -24,6 +24,7 @@ IRQ5 = $74 IRQ6 = $78 IRQ7 = $7C + move.l a5,-(sp) ; a5 is the frame pointer register, but it is needed below for Supervisor() move.l 4.w,a6 jsr exec.Forbid(a6) ; Do not run other tasks btst.b #0,exec_AttnFlags+1(a6) ; Check if > 68000 processor @@ -65,7 +66,8 @@ IRQ7 = $7C .getvbr: dc.l $4E7A0801 ; MOVEC VBR,d0 - privileged instruction rte -.skip: rts +.skip: move.l (sp)+,a5 ; restore the caller's frame pointer + rts }} } diff --git a/compiler/res/prog8lib/shared_m68k_adpcm.p8 b/compiler/res/prog8lib/shared_m68k_adpcm.p8 index c28bde93e..c0d251068 100644 --- a/compiler/res/prog8lib/shared_m68k_adpcm.p8 +++ b/compiler/res/prog8lib/shared_m68k_adpcm.p8 @@ -284,12 +284,14 @@ adpcm { decode_block_stereo_loop(nibblesptr, outptr) } - private asmsub decode_block_stereo_loop(pointer nibblesptr @A0, pointer outptr @A1) clobbers (D0, D1, D2, D3, D4, D5, D6, D7, A0, A1, A2, A3, A4, A5) { + private asmsub decode_block_stereo_loop(pointer nibblesptr @A0, pointer outptr @A1) clobbers (D0, D1, D2, D3, D4, D5, D6, D7, A0, A1, A2, A3, A4) { ; Decode 248 nibbles in 31 outer iterations, each processing 4 left bytes ; then 4 right bytes. D7/D6 = left predict/(rowindex+1)*32, D5/D3 = right, ; D4 = loaded longword, D2 = temp word, A5 = outer loop counter. ; Left samples go to (a1)/4(a1) and right samples to 2(a1)/6(a1) via a4, ; producing interleaved L,R output directly with no separate interleave pass. + ; A5 is used as scratch here, but it is the caller's frame pointer register, so it is + ; saved and restored around the loop. %asm {{ lea p8b_adpcm.p8v_deltas_table-32,a2 lea p8b_adpcm.p8v_next_state_table,a3 @@ -303,6 +305,7 @@ adpcm { addq.w #1,d6 lsl.w #5,d6 ; S_left = (rowindex + 1) * 32 move.w p8b_adpcm.p8v_predict,d7 + move.l a5,-(sp) ; preserve the caller's frame pointer register suba.l a5,a5 move.w #31,a5 .loop: @@ -508,6 +511,7 @@ adpcm { lsr.w #5,d1 subq.w #1,d1 move.b d1,p8b_adpcm.p8v_rowindex_2 + move.l (sp)+,a5 ; restore the caller's frame pointer register rts }} } diff --git a/compiler/test/codegeneration/TestM68k.kt b/compiler/test/codegeneration/TestM68k.kt index ca4fb254e..5c2f16462 100644 --- a/compiler/test/codegeneration/TestM68k.kt +++ b/compiler/test/codegeneration/TestM68k.kt @@ -519,4 +519,125 @@ main { startIdx shouldBeGreaterThan -1 (sizeIdx < startIdx) shouldBe true } + + test("subroutine calling assembly that overwrites a5 keeps the static convention") { + // A5 is the frame pointer of a framed subroutine. An asmsub that clobbers it without + // preserving it would silently invalidate the caller's frame slots (and its `unlk a5`), + // so such a caller must not be given a stack frame at all. + val src = """ +main { + inline asmsub clobberframepointer() { + %asm {{ + moveq #1, d0 + lea.l some_label(PC),a5 +some_label: + rts + }} + } + + sub start() { + ubyte localvar + clobberframepointer() + localvar = 1 + } +} +""" + val result = compileText(Amiga500Target(), optimize = false, src, outputDir, writeAssembly = true, assemble = false) + result shouldNotBe null + val asm = result!!.compilationOptions.outputDir.resolve("${result.compilerAst.name}.asm").toFile().readText() + val start = asm.indexOf("; ---- Subroutine: p8b_main.p8s_start") + val end = asm.indexOf("; End of subroutine: p8b_main.p8s_start") + val lines = asm.substring(start, end).lines().map { it.trim() } + lines.none { it.startsWith("link a5") } shouldBe true + lines.none { it.startsWith("unlk") } shouldBe true + // the local keeps its static cell, so no frame-relative access at all + lines.any { it.contains("p8b_main.p8s_start.p8v_localvar") } shouldBe true + } + + test("subroutine calling assembly that preserves a5 gets a stack frame") { + // the same program, but the asmsub saves and restores a5, so the frame stays valid + val src = """ +main { + inline asmsub preserveframepointer() { + %asm {{ + move.l a5,-(sp) + moveq #1, d0 + lea.l some_label(PC),a5 +some_label: + move.l (sp)+,a5 + rts + }} + } + + sub start() { + ubyte localvar + preserveframepointer() + localvar = 1 + } +} +""" + val result = compileText(Amiga500Target(), optimize = false, src, outputDir, writeAssembly = true, assemble = false) + result shouldNotBe null + val asm = result!!.compilationOptions.outputDir.resolve("${result.compilerAst.name}.asm").toFile().readText() + val start = asm.indexOf("; ---- Subroutine: p8b_main.p8s_start") + val end = asm.indexOf("; End of subroutine: p8b_main.p8s_start") + val lines = asm.substring(start, end).lines().map { it.trim() } + lines.count { it.startsWith("link a5") } shouldBe 1 + lines.count { it.startsWith("unlk") } shouldBe 1 + lines.any { it.contains("-1(a5)") } shouldBe true + } + + test("custom.grab_system preserves the frame pointer register a5") { + // grab_system needs a5 to pass the VBR-reading helper to exec.Supervisor(), and A5 is now + // the frame pointer of the calling subroutine, so it must be saved and restored. + val src = """ +%import custom + +main { + sub start() { + custom.grab_system() + custom.restore_system() + } +} +""" + val result = compileText(Amiga500Target(), optimize = false, src, outputDir, writeAssembly = true, assemble = false) + result shouldNotBe null + val asm = result!!.compilationOptions.outputDir.resolve("${result.compilerAst.name}.asm").toFile().readText() + val grab = asm.substring(asm.indexOf("custom.grab_system:"), asm.indexOf("; End of subroutine: custom.grab_system")) + grab.lines().map { it.substringBefore(';').trim() }.let { lines -> + lines.count { it == "move.l a5,-(sp)" } shouldBe 1 + lines.count { it == ".skip: move.l (sp)+,a5" } shouldBe 1 + } + } + + test("adpcm stereo block decoder preserves the frame pointer register a5") { + // the stereo loop kernel uses A5 as its outer loop counter, but A5 is now the frame pointer + // of every framed Prog8 subroutine, so it must save and restore it around the loop. + val src = """ +%import adpcm + +main { + pointer inbuf = memory("inbuf", 256, 0) + pointer outbuf = memory("outbuf", 1024, 0) + + sub start() { + adpcm.decode_block_stereo(inbuf, outbuf) + } +} +""" + val result = compileText(Amiga500Target(), optimize = false, src, outputDir, writeAssembly = true, assemble = false) + result shouldNotBe null + val asm = result!!.compilationOptions.outputDir.resolve("${result.compilerAst.name}.asm").toFile().readText() + val kernel = asm.substring( + asm.indexOf("p8b_adpcm.p8s_decode_block_stereo_loop:"), + asm.indexOf("; End of subroutine: p8b_adpcm.p8s_decode_block_stereo_loop") + ) + kernel.lines().map { it.substringBefore(';').trim() }.let { lines -> + lines.count { it == "move.l a5,-(sp)" } shouldBe 1 + lines.count { it == "move.l (sp)+,a5" } shouldBe 1 + } + // its Prog8 caller and everything above it keep their stack frames + asm.contains("; ---- Subroutine: p8b_adpcm.p8s_decode_block_stereo ----") shouldBe true + asm.contains("link a5,#") shouldBe true + } }) diff --git a/docs/source/technical.rst b/docs/source/technical.rst index 8a29a38df..01d5ddd3c 100644 --- a/docs/source/technical.rst +++ b/docs/source/technical.rst @@ -280,6 +280,11 @@ assembly that calls a normal Prog8 subroutine must follow the stack convention a ``on..goto`` and ``on..call`` jump tables remain static because their symbolic initializers cannot currently be placed in a frame. + Assembly code that writes to ``A5`` without preserving the caller's frame pointer would + invalidate every frame slot of its caller, so a subroutine that (directly or indirectly) calls + such assembly is not given a stack frame and keeps the static convention instead. The usual + ``move.l a5,-(sp)`` ... ``move.l (sp)+,a5`` prologue and epilogue is recognised as balanced. + A single local frame is limited to 16 KiB. The compiler reports oversized frames, but does not detect exhaustion caused by a deep call chain or recursion. Frame-slot reuse is also conservative in routines containing loops, so such routines may use more stack space than diff --git a/examples/amiga/lowlevel-timer.p8 b/examples/amiga/lowlevel-timer.p8 index 8e57d6fa3..0da6beedd 100644 --- a/examples/amiga/lowlevel-timer.p8 +++ b/examples/amiga/lowlevel-timer.p8 @@ -139,4 +139,4 @@ main { txt.print("clean exit\n") sys.exit(0) } -} +} \ No newline at end of file