From d32b70bb75c58dbf2335756dbd2175f20aa765d0 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Thu, 8 Oct 2026 20:50:39 +0200 Subject: [PATCH] m68k: tear down frames on external conditional branches --- .../src/prog8/codegen/m68k/InstrBranch.kt | 87 ++++++++++++------- codeGenM68k/test/TestStackFrameEmission.kt | 45 ++++++++++ 2 files changed, 101 insertions(+), 31 deletions(-) diff --git a/codeGenM68k/src/prog8/codegen/m68k/InstrBranch.kt b/codeGenM68k/src/prog8/codegen/m68k/InstrBranch.kt index 0d660f6dd..7b080863d 100644 --- a/codeGenM68k/src/prog8/codegen/m68k/InstrBranch.kt +++ b/codeGenM68k/src/prog8/codegen/m68k/InstrBranch.kt @@ -28,40 +28,46 @@ import prog8.intermediate.requireSrcB import prog8.intermediate.requireTarget internal fun AsmGen.translateBranch(insn: IRInstruction) { - val label: String = when (val target = insn.requireTarget()) { + val target = insn.requireTarget() + val label: String = when (target) { is CodeReference.Label -> fixNameSymbols(target.name) is CodeReference.Absolute -> target.address.toHex() is CodeReference.Indirect -> error("branch needs a static target") } + val leavesSubroutine = when (target) { + is CodeReference.Label -> labelIsOutsideCurrentSub(target.name) + is CodeReference.Absolute -> true + is CodeReference.Indirect -> false + } when (insn.opcode) { - Opcode.BSTCC -> emitBranch("bcc", label) - Opcode.BSTCS -> emitBranch("bcs", label) - Opcode.BSTEQ -> emitBranch("beq", label) - Opcode.BSTNE -> emitBranch("bne", label) - Opcode.BSTNEG -> emitBranch("bmi", label) - Opcode.BSTPOS -> emitBranch("bpl", label) - Opcode.BSTVC -> emitBranch("bvc", label) - Opcode.BSTVS -> emitBranch("bvs", label) + Opcode.BSTCC -> emitBranch("bcc", label, leavesSubroutine) + Opcode.BSTCS -> emitBranch("bcs", label, leavesSubroutine) + Opcode.BSTEQ -> emitBranch("beq", label, leavesSubroutine) + Opcode.BSTNE -> emitBranch("bne", label, leavesSubroutine) + Opcode.BSTNEG -> emitBranch("bmi", label, leavesSubroutine) + Opcode.BSTPOS -> emitBranch("bpl", label, leavesSubroutine) + Opcode.BSTVC -> emitBranch("bvc", label, leavesSubroutine) + Opcode.BSTVS -> emitBranch("bvs", label, leavesSubroutine) // Unsigned integer comparison branches - Opcode.BGT -> cmpBranchUnsignedImm(insn, label, "bhi") - Opcode.BGE -> cmpBranchUnsignedImm(insn, label, "bhs") - Opcode.BLT -> cmpBranchUnsignedImm(insn, label, "blo") - Opcode.BLE -> cmpBranchUnsignedImm(insn, label, "bls") + Opcode.BGT -> cmpBranchUnsignedImm(insn, label, "bhi", leavesSubroutine) + Opcode.BGE -> cmpBranchUnsignedImm(insn, label, "bhs", leavesSubroutine) + Opcode.BLT -> cmpBranchUnsignedImm(insn, label, "blo", leavesSubroutine) + Opcode.BLE -> cmpBranchUnsignedImm(insn, label, "bls", leavesSubroutine) - Opcode.BGTR -> cmpBranchUnsignedReg(insn, label, "bhi") - Opcode.BGER -> cmpBranchUnsignedReg(insn, label, "bhs") + Opcode.BGTR -> cmpBranchUnsignedReg(insn, label, "bhi", leavesSubroutine) + Opcode.BGER -> cmpBranchUnsignedReg(insn, label, "bhs", leavesSubroutine) // BLTR doesn't exist in IR — uses BGTR with swapped operands // Signed integer comparison branches - Opcode.BGTS -> cmpBranchSignedImm(insn, label, "bgt") - Opcode.BGES -> cmpBranchSignedImm(insn, label, "bge") - Opcode.BLTS -> cmpBranchSignedImm(insn, label, "blt") - Opcode.BLES -> cmpBranchSignedImm(insn, label, "ble") + Opcode.BGTS -> cmpBranchSignedImm(insn, label, "bgt", leavesSubroutine) + Opcode.BGES -> cmpBranchSignedImm(insn, label, "bge", leavesSubroutine) + Opcode.BLTS -> cmpBranchSignedImm(insn, label, "blt", leavesSubroutine) + Opcode.BLES -> cmpBranchSignedImm(insn, label, "ble", leavesSubroutine) - Opcode.BGTSR -> cmpBranchSignedReg(insn, label, "bgt") - Opcode.BGESR -> cmpBranchSignedReg(insn, label, "bge") + Opcode.BGTSR -> cmpBranchSignedReg(insn, label, "bgt", leavesSubroutine) + Opcode.BGESR -> cmpBranchSignedReg(insn, label, "bge", leavesSubroutine) // BLTSR doesn't exist in IR — uses BGTSR with swapped operands else -> error("Unknown branch opcode: ${insn.opcode}") @@ -70,7 +76,7 @@ internal fun AsmGen.translateBranch(insn: IRInstruction) { // === Unsigned comparisons: register vs immediate === -private fun AsmGen.cmpBranchUnsignedImm(insn: IRInstruction, label: String, branchOp: String) { +private fun AsmGen.cmpBranchUnsignedImm(insn: IRInstruction, label: String, branchOp: String, leavesSubroutine: Boolean) { val type = insn.type ?: IRDataType.BYTE val reg = insn.requireSrcA().intNumber val imm = insn.requireImmediateInt() @@ -81,24 +87,24 @@ private fun AsmGen.cmpBranchUnsignedImm(insn: IRInstruction, label: String, bran emitLine("tst$s ${regAddr(reg)}") else emitLine("cmpi$s #$imm, ${regAddr(reg)}") - emitBranch(branchOp, label) + emitBranch(branchOp, label, leavesSubroutine) } // === Unsigned comparisons: register vs register === -private fun AsmGen.cmpBranchUnsignedReg(insn: IRInstruction, label: String, branchOp: String) { +private fun AsmGen.cmpBranchUnsignedReg(insn: IRInstruction, label: String, branchOp: String, leavesSubroutine: Boolean) { val type = insn.type ?: IRDataType.BYTE val left = insn.requireSrcA().intNumber val right = insn.requireSrcB().intNumber val s = dtSuffix(type) emitLoadD0(left, type) emitLine("cmp$s ${regAddr(right)}, d0") - emitBranch(branchOp, label) + emitBranch(branchOp, label, leavesSubroutine) } // === Signed comparisons: register vs immediate === -private fun AsmGen.cmpBranchSignedImm(insn: IRInstruction, label: String, branchOp: String) { +private fun AsmGen.cmpBranchSignedImm(insn: IRInstruction, label: String, branchOp: String, leavesSubroutine: Boolean) { val type = insn.type ?: IRDataType.BYTE val reg = insn.requireSrcA().intNumber val imm = insn.requireImmediateInt() @@ -109,22 +115,41 @@ private fun AsmGen.cmpBranchSignedImm(insn: IRInstruction, label: String, branch emitLine("tst$s ${regAddr(reg)}") else emitLine("cmpi$s #$imm, ${regAddr(reg)}") - emitBranch(branchOp, label) + emitBranch(branchOp, label, leavesSubroutine) } // === Signed comparisons: register vs register === -private fun AsmGen.cmpBranchSignedReg(insn: IRInstruction, label: String, branchOp: String) { +private fun AsmGen.cmpBranchSignedReg(insn: IRInstruction, label: String, branchOp: String, leavesSubroutine: Boolean) { val type = insn.type ?: IRDataType.BYTE val left = insn.requireSrcA().intNumber val right = insn.requireSrcB().intNumber val s = dtSuffix(type) emitLoadD0(left, type) emitLine("cmp$s ${regAddr(right)}, d0") - emitBranch(branchOp, label) + emitBranch(branchOp, label, leavesSubroutine) } -private fun AsmGen.emitBranch(branchOp: String, label: String) { +private val inverseBranchOps = mapOf( + "bcc" to "bcs", "bcs" to "bcc", + "beq" to "bne", "bne" to "beq", + "bmi" to "bpl", "bpl" to "bmi", + "bvc" to "bvs", "bvs" to "bvc", + "bhi" to "bls", "bls" to "bhi", + "bhs" to "blo", "blo" to "bhs", + "bgt" to "ble", "ble" to "bgt", + "bge" to "blt", "blt" to "bge") + +private fun AsmGen.emitBranch(branchOp: String, label: String, leavesSubroutine: Boolean) { invalidateD0Cache() - emitLine("$branchOp $label") + if (!leavesSubroutine || !frameActive) { + emitLine("$branchOp $label") + return + } + + val skipLabel = makeLabel("branch_keep_frame") + emitLine("${inverseBranchOps.getValue(branchOp)} $skipLabel") + emitFrameUnlk() + emitLine("jmp $label") + emitLabel(skipLabel) } diff --git a/codeGenM68k/test/TestStackFrameEmission.kt b/codeGenM68k/test/TestStackFrameEmission.kt index 260bc7d6e..215caa159 100644 --- a/codeGenM68k/test/TestStackFrameEmission.kt +++ b/codeGenM68k/test/TestStackFrameEmission.kt @@ -377,6 +377,51 @@ class TestStackFrameEmission : FunSpec({ lines.none { it.startsWith("unlk") } shouldBe true } + test("conditional branch to an absolute address tears down only the taken path") { + val lines = generateAsm(16, listOf( + IRInstructions.branch(Opcode.BSTEQ, codeAddress(0x1234u)), + IRInstructions.simple(Opcode.RETURN) + )) + val branchIdx = lines.indexOfFirst { it.startsWith("bne") } + val jmpIdx = lines.indexOfFirst { it.startsWith("jmp") && "$1234" in it } + (branchIdx >= 0) shouldBe true + (jmpIdx > branchIdx) shouldBe true + lines[jmpIdx - 1] shouldBe "unlk a5" + lines.count { it == "unlk a5" } shouldBe 2 + } + + test("conditional branch to an external non-entry label tears down only the taken path") { + val outsideInner = IRCodeChunk("test.outside.inner", null).also { + it.instructions.add(IRInstructions.simple(Opcode.RETURN)) + } + val lines = generateAsmForSubs( + sub("test.start", frameSize = 16, instructions = listOf( + IRInstructions.branch(Opcode.BSTEQ, CodeReference.Label("test.outside.inner")), + IRInstructions.simple(Opcode.RETURN) + )), + sub("test.outside", frameSize = 0, instructions = listOf(IRInstructions.simple(Opcode.RETURN)), + extraChunks = listOf(outsideInner)) + ) + val jmpIdx = lines.indexOfFirst { it.startsWith("jmp") && "outside.inner" in it } + (jmpIdx > 0) shouldBe true + lines[jmpIdx - 1] shouldBe "unlk a5" + lines.any { it.startsWith("bne") } shouldBe true + } + + test("signed comparison branch to an absolute address tears down only the taken path") { + val lines = generateAsm(16, listOf( + IRInstructions.branchImmediate(Opcode.BGTS, IRDataType.WORD, 1, 1, codeAddress(0x1234u)), + IRInstructions.simple(Opcode.RETURN) + )) + lines.any { it.startsWith("cmpi.w") } shouldBe true + val branchIdx = lines.indexOfFirst { it.startsWith("ble") } + val jmpIdx = lines.indexOfFirst { it.startsWith("jmp") && "$1234" in it } + (branchIdx >= 0) shouldBe true + (jmpIdx > branchIdx) shouldBe true + lines[jmpIdx - 1] shouldBe "unlk a5" + lines.count { it == "unlk a5" } shouldBe 2 + } + test("jump to a label in another subroutine pops the frame before jumping") { // jumping out of the subroutine is a documented way to end it, so our frame must not be // left behind for whatever runs next (and its eventual rts) to trip over