From 4b4af9b5271ddd25aaf74624e43f2c8a0b0d84f7 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sun, 27 Oct 2024 13:49:00 +0100 Subject: [PATCH] no longer silently add RTS to asmsubs that don't have one --- .../src/prog8/code/core/CompilationOptions.kt | 1 + .../src/prog8/codegen/cpu6502/AsmGen.kt | 6 +-- compiler/res/prog8lib/atari/syslib.p8 | 2 + compiler/res/prog8lib/c128/syslib.p8 | 4 +- compiler/res/prog8lib/c128/textio.p8 | 2 + compiler/res/prog8lib/c64/graphics.p8 | 1 + compiler/res/prog8lib/c64/syslib.p8 | 2 + compiler/res/prog8lib/c64/textio.p8 | 2 + compiler/res/prog8lib/conv.p8 | 7 ++- compiler/res/prog8lib/cx16/floats.p8 | 1 + compiler/res/prog8lib/cx16/syslib.p8 | 8 ++- compiler/res/prog8lib/math.p8 | 9 +++- compiler/res/prog8lib/pet32/syslib.p8 | 2 + compiler/res/prog8lib/pet32/textio.p8 | 1 + .../prog8lib/shared_cbm_textio_functions.p8 | 1 + compiler/res/prog8lib/string.p8 | 2 +- compiler/src/prog8/CompilerMain.kt | 3 ++ compiler/src/prog8/compiler/Compiler.kt | 2 + .../compiler/astprocessing/AstExtensions.kt | 26 +++++++-- .../astprocessing/BeforeAsmAstChanger.kt | 25 ++++++--- compiler/test/TestCompilerOnExamples.kt | 1 + compiler/test/TestCompilerOptionLibdirs.kt | 1 + compiler/test/TestOptimization.kt | 1 + compiler/test/TestSubroutines.kt | 5 +- .../test/codegeneration/TestVariousCodeGen.kt | 49 +++++++++++++++++ compiler/test/helpers/compileXyz.kt | 1 + docs/source/compiling.rst | 6 +++ docs/source/todo.rst | 3 +- examples/cx16/pcmaudio/stream-simple-aflow.p8 | 2 +- examples/cx16/pcmaudio/stream-wav.p8 | 3 +- examples/cx16/pcmaudio/vumeter.p8 | 2 +- examples/test.p8 | 53 +++++-------------- 32 files changed, 167 insertions(+), 67 deletions(-) diff --git a/codeCore/src/prog8/code/core/CompilationOptions.kt b/codeCore/src/prog8/code/core/CompilationOptions.kt index e403498f1..36d01f946 100644 --- a/codeCore/src/prog8/code/core/CompilationOptions.kt +++ b/codeCore/src/prog8/code/core/CompilationOptions.kt @@ -27,6 +27,7 @@ class CompilationOptions(val output: OutputType, var slabsHighBank: Int? = null, var slabsGolden: Boolean = false, var splitWordArrays: Boolean = false, + var addMissingRts: Boolean = false, // deprecated, will likely go way in future version var breakpointCpuInstruction: String? = null, var outputDir: Path = Path(""), var symbolDefs: Map = emptyMap() diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt index 65d6894b2..991a05790 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt @@ -1040,7 +1040,7 @@ $repeatLabel""") } } - private fun translate(ret: PtReturn, withRts: Boolean=true) { + private fun translate(ret: PtReturn) { ret.value?.let { returnvalue -> val sub = ret.definingSub()!! val returnReg = sub.returnRegister()!! @@ -1057,9 +1057,7 @@ $repeatLabel""") } } } - - if(withRts) - out(" rts") + out(" rts") } private fun translate(asm: PtInlineAssembly) { diff --git a/compiler/res/prog8lib/atari/syslib.p8 b/compiler/res/prog8lib/atari/syslib.p8 index bd4954f39..a7134174b 100644 --- a/compiler/res/prog8lib/atari/syslib.p8 +++ b/compiler/res/prog8lib/atari/syslib.p8 @@ -248,6 +248,7 @@ save_SCRATCH_ZPB1 .byte 0 save_SCRATCH_ZPREG .byte 0 save_SCRATCH_ZPWORD1 .word 0 save_SCRATCH_ZPWORD2 .word 0 + ; !notreached! }} } @@ -461,6 +462,7 @@ cx16 { _cx16_vreg_storage .word 0,0,0,0,0,0,0,0 .word 0,0,0,0,0,0,0,0 + ; !notreached! }} } diff --git a/compiler/res/prog8lib/c128/syslib.p8 b/compiler/res/prog8lib/c128/syslib.p8 index cc670c73f..0ddf29091 100644 --- a/compiler/res/prog8lib/c128/syslib.p8 +++ b/compiler/res/prog8lib/c128/syslib.p8 @@ -453,6 +453,7 @@ save_SCRATCH_ZPB1 .byte 0 save_SCRATCH_ZPREG .byte 0 save_SCRATCH_ZPWORD1 .word 0 save_SCRATCH_ZPWORD2 .word 0 + ; !notreached! }} } @@ -504,8 +505,6 @@ _modified tax pla rti - -_use_kernal .byte 0 }} } @@ -978,6 +977,7 @@ cx16 { _cx16_vreg_storage .word 0,0,0,0,0,0,0,0 .word 0,0,0,0,0,0,0,0 + ; !notreached! }} } diff --git a/compiler/res/prog8lib/c128/textio.p8 b/compiler/res/prog8lib/c128/textio.p8 index 8a62ba700..bc089f81c 100644 --- a/compiler/res/prog8lib/c128/textio.p8 +++ b/compiler/res/prog8lib/c128/textio.p8 @@ -314,6 +314,7 @@ _mod sta $ffff ; modified rts _screenrows .word cbm.Screen + range(0, 1000, 40) + ; !notreached! }} } @@ -357,6 +358,7 @@ _mod sta $ffff ; modified rts _colorrows .word $d800 + range(0, 1000, 40) + ; !notreached! }} } diff --git a/compiler/res/prog8lib/c64/graphics.p8 b/compiler/res/prog8lib/c64/graphics.p8 index 8921fecef..8507c4779 100644 --- a/compiler/res/prog8lib/c64/graphics.p8 +++ b/compiler/res/prog8lib/c64/graphics.p8 @@ -386,6 +386,7 @@ _plot_y_values := p8c_BITMAP_ADDRESS + 320*(range(200)>>3) + (range(200) & 7) _y_lookup_lo .byte <_plot_y_values _y_lookup_hi .byte >_plot_y_values + ; !notreached! }} } diff --git a/compiler/res/prog8lib/c64/syslib.p8 b/compiler/res/prog8lib/c64/syslib.p8 index cb3273724..4b0ffb415 100644 --- a/compiler/res/prog8lib/c64/syslib.p8 +++ b/compiler/res/prog8lib/c64/syslib.p8 @@ -451,6 +451,7 @@ save_SCRATCH_ZPB1 .byte 0 save_SCRATCH_ZPREG .byte 0 save_SCRATCH_ZPWORD1 .word 0 save_SCRATCH_ZPWORD2 .word 0 + ; !notreached! }} } @@ -975,6 +976,7 @@ cx16 { _cx16_vreg_storage .word 0,0,0,0,0,0,0,0 .word 0,0,0,0,0,0,0,0 + ; !notreached! }} } diff --git a/compiler/res/prog8lib/c64/textio.p8 b/compiler/res/prog8lib/c64/textio.p8 index 9316aaa03..5e186aae1 100644 --- a/compiler/res/prog8lib/c64/textio.p8 +++ b/compiler/res/prog8lib/c64/textio.p8 @@ -317,6 +317,7 @@ _mod sta $ffff ; modified rts _screenrows .word cbm.Screen + range(0, 1000, 40) + ; !notreached! }} } @@ -360,6 +361,7 @@ _mod sta $ffff ; modified rts _colorrows .word $d800 + range(0, 1000, 40) + ; !notreached! }} } diff --git a/compiler/res/prog8lib/conv.p8 b/compiler/res/prog8lib/conv.p8 index 756855a71..35ef58af2 100644 --- a/compiler/res/prog8lib/conv.p8 +++ b/compiler/res/prog8lib/conv.p8 @@ -200,6 +200,7 @@ _allzero lda #'0' sta string_out,x inx bne _end + ; !notreached! }} } @@ -402,6 +403,7 @@ _digit bne _parse ; never reached _negative .byte 0 + ; !notreached! }} } @@ -472,6 +474,7 @@ _try_iso bcs _stop and #63 bne _add_letter + ; !notreached! }} } @@ -702,7 +705,7 @@ decHundreds .byte 0 decTens .byte 0 decOnes .byte 0 .byte 0 ; zero-terminate the decimal output string - + ; !notreached! }} } @@ -736,6 +739,7 @@ asmsub internal_ubyte2hex (ubyte value @A) clobbers(X) -> ubyte @A, ubyte @Y rts _hex_digits .text "0123456789abcdef" ; can probably be reused for other stuff as well + ; !notreached! }} } @@ -753,6 +757,7 @@ asmsub internal_uword2hex (uword value @AY) clobbers(A,Y) { sty output+3 rts output .text "0000", $00 ; 0-terminated output buffer (to make printing easier) + ; !notreached! }} } diff --git a/compiler/res/prog8lib/cx16/floats.p8 b/compiler/res/prog8lib/cx16/floats.p8 index 0584936f1..f5e32e4c8 100644 --- a/compiler/res/prog8lib/cx16/floats.p8 +++ b/compiler/res/prog8lib/cx16/floats.p8 @@ -186,6 +186,7 @@ _borked + jmp sys.exit _msg .text 13,"?rom 47+ required for val1",13,0 + ; !notreached! }} } diff --git a/compiler/res/prog8lib/cx16/syslib.p8 b/compiler/res/prog8lib/cx16/syslib.p8 index 71b1a917a..b47c33d7f 100644 --- a/compiler/res/prog8lib/cx16/syslib.p8 +++ b/compiler/res/prog8lib/cx16/syslib.p8 @@ -838,6 +838,7 @@ _large ora cx16.r2L + lda #0 rts _strides_lsb .byte 0,1,2,4,8,16,32,64,128,255,255,40,80,160,255,255 + ; !notreached! }} } @@ -955,6 +956,7 @@ asmsub save_virtual_registers() clobbers(A,Y) { _cx16_vreg_storage .word 0,0,0,0,0,0,0,0 .word 0,0,0,0,0,0,0,0 + ; !notreached! }} } @@ -993,6 +995,7 @@ asmsub save_vera_context() clobbers(A) { sta _vera_storage+6 rts _vera_storage: .byte 0,0,0,0,0,0,0,0 + ; !notreached! }} } @@ -1315,6 +1318,7 @@ _continue iny bne _continue + pla sta $00 + rts }} } @@ -1499,7 +1503,8 @@ asmsub restore_irq() clobbers(A) { cli rts _orig_irqvec .word 0 - }} + ; !notreached! + }} } asmsub set_rasterirq(uword handler @AY, uword rasterpos @R0) clobbers(A) { @@ -1775,6 +1780,7 @@ save_SCRATCH_ZPB1 .byte 0 save_SCRATCH_ZPREG .byte 0 save_SCRATCH_ZPWORD1 .word 0 save_SCRATCH_ZPWORD2 .word 0 + ; !notreached! }} } diff --git a/compiler/res/prog8lib/math.p8 b/compiler/res/prog8lib/math.p8 index 5c33bae41..fdc461155 100644 --- a/compiler/res/prog8lib/math.p8 +++ b/compiler/res/prog8lib/math.p8 @@ -13,6 +13,7 @@ math { lda _sinecos8u,y rts _sinecos8u .byte trunc(128.0 + 127.5 * sin(range(256+64) * rad(360.0/256.0))) + ; !notreached! }} } @@ -30,6 +31,7 @@ _sinecos8u .byte trunc(128.0 + 127.5 * sin(range(256+64) * rad(360.0/256.0))) lda _sinecos8,y rts _sinecos8 .char trunc(127.0 * sin(range(256+64) * rad(360.0/256.0))) + ; !notreached! }} } @@ -47,6 +49,7 @@ _sinecos8 .char trunc(127.0 * sin(range(256+64) * rad(360.0/256.0))) lda _sinecosR8u,y rts _sinecosR8u .byte trunc(128.0 + 127.5 * sin(range(180+45) * rad(360.0/180.0))) + ; !notreached! }} } @@ -64,6 +67,7 @@ _sinecosR8u .byte trunc(128.0 + 127.5 * sin(range(180+45) * rad(360.0/180.0))) lda _sinecosR8,y rts _sinecosR8 .char trunc(127.0 * sin(range(180+45) * rad(360.0/180.0))) + ; !notreached! }} } @@ -131,6 +135,7 @@ _sinecosR8 .char trunc(127.0 * sin(range(180+45) * rad(360.0/180.0))) rts + lsr a bne - + ; !notreached! }} } @@ -331,7 +336,7 @@ _quadrant_region_to_direction: .byte 8, 4,16,20 .byte 7, 5,17,19 .byte 6, 6,18,18 - + ; !notreached! }} } @@ -464,7 +469,7 @@ log2_tab .byte $fb,$fb,$fb,$fc,$fc,$fc,$fc,$fc .byte $fd,$fd,$fd,$fd,$fd,$fd,$fe,$fe .byte $fe,$fe,$fe,$ff,$ff,$ff,$ff,$ff - + ; !notreached! }} } diff --git a/compiler/res/prog8lib/pet32/syslib.p8 b/compiler/res/prog8lib/pet32/syslib.p8 index 02b32dbce..5bf2201c4 100644 --- a/compiler/res/prog8lib/pet32/syslib.p8 +++ b/compiler/res/prog8lib/pet32/syslib.p8 @@ -350,6 +350,7 @@ save_SCRATCH_ZPB1 .byte 0 save_SCRATCH_ZPREG .byte 0 save_SCRATCH_ZPWORD1 .word 0 save_SCRATCH_ZPWORD2 .word 0 + ; !notreached! }} } @@ -564,6 +565,7 @@ cx16 { _cx16_vreg_storage .word 0,0,0,0,0,0,0,0 .word 0,0,0,0,0,0,0,0 + ; !notreached! }} } diff --git a/compiler/res/prog8lib/pet32/textio.p8 b/compiler/res/prog8lib/pet32/textio.p8 index da60aeacd..2f144e934 100644 --- a/compiler/res/prog8lib/pet32/textio.p8 +++ b/compiler/res/prog8lib/pet32/textio.p8 @@ -171,6 +171,7 @@ _mod sta $ffff ; modified rts _screenrows .word cbm.Screen + range(0, 1000, 40) + ; !notreached! }} } diff --git a/compiler/res/prog8lib/shared_cbm_textio_functions.p8 b/compiler/res/prog8lib/shared_cbm_textio_functions.p8 index 12faa9109..96bf03143 100644 --- a/compiler/res/prog8lib/shared_cbm_textio_functions.p8 +++ b/compiler/res/prog8lib/shared_cbm_textio_functions.p8 @@ -171,6 +171,7 @@ _allzero lda #'0' eor P8ZP_SCRATCH_REG rts _offsets .byte 128, 0, 64, 32, 64, 192, 128, 128 + ; !notreached! }} } diff --git a/compiler/res/prog8lib/string.p8 b/compiler/res/prog8lib/string.p8 index ca14aefca..447f7f0a0 100644 --- a/compiler/res/prog8lib/string.p8 +++ b/compiler/res/prog8lib/string.p8 @@ -178,7 +178,7 @@ _found tya rts _str .word 0 - + ; !notreached! }} } diff --git a/compiler/src/prog8/CompilerMain.kt b/compiler/src/prog8/CompilerMain.kt index 6e120aaed..12ff32d7f 100644 --- a/compiler/src/prog8/CompilerMain.kt +++ b/compiler/src/prog8/CompilerMain.kt @@ -54,6 +54,7 @@ private fun compileMain(args: Array): Boolean { val outputDir by cli.option(ArgType.String, fullName = "out", description = "directory for output files instead of current directory").default(".") val quietAssembler by cli.option(ArgType.Boolean, fullName = "quietasm", description = "don't print assembler output results") val warnSymbolShadowing by cli.option(ArgType.Boolean, fullName = "warnshadow", description="show assembler warnings about symbol shadowing") + val addMissingRts by cli.option(ArgType.Boolean, fullName = "addmissingrts", description="enable old behavior that silently adds RTS to asmsubs that don't have one (deprecated, may go away in future version)") val sourceDirs by cli.option(ArgType.String, fullName="srcdirs", description = "list of extra paths, separated with ${File.pathSeparator}, to search in for imported modules").multiple().delimiter(File.pathSeparator) val includeSourcelines by cli.option(ArgType.Boolean, fullName = "sourcelines", description = "include original Prog8 source lines in generated asm code") val splitWordArrays by cli.option(ArgType.Boolean, fullName = "splitarrays", description = "treat all word arrays as tagged with @split to make them lsb/msb split in memory") @@ -179,6 +180,7 @@ private fun compileMain(args: Array): Boolean { slabsGolden == true, compilationTarget!!, splitWordArrays == true, + addMissingRts == true, breakpointCpuInstruction, printAst1 == true, printAst2 == true, @@ -259,6 +261,7 @@ private fun compileMain(args: Array): Boolean { slabsGolden == true, compilationTarget!!, splitWordArrays == true, + addMissingRts == true, breakpointCpuInstruction, printAst1 == true, printAst2 == true, diff --git a/compiler/src/prog8/compiler/Compiler.kt b/compiler/src/prog8/compiler/Compiler.kt index 8c0be66ca..6b439fe56 100644 --- a/compiler/src/prog8/compiler/Compiler.kt +++ b/compiler/src/prog8/compiler/Compiler.kt @@ -52,6 +52,7 @@ class CompilerArguments(val filepath: Path, val slabsGolden: Boolean, val compilationTarget: String, val splitWordArrays: Boolean, + val addMissingRts: Boolean, val breakpointCpuInstruction: String?, val printAst1: Boolean, val printAst2: Boolean, @@ -89,6 +90,7 @@ fun compileProgram(args: CompilerArguments): CompilationResult? { slabsHighBank = args.slabsHighBank slabsGolden = args.slabsGolden splitWordArrays = args.splitWordArrays + addMissingRts = args.addMissingRts outputDir = args.outputDir.normalize() symbolDefs = args.symbolDefs } diff --git a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt index 5c79149cd..226c04d73 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt @@ -22,7 +22,7 @@ internal fun Program.checkValid(errors: IErrorReporter, compilerOptions: Compila } internal fun Program.processAstBeforeAsmGeneration(compilerOptions: CompilationOptions, errors: IErrorReporter) { - val fixer = BeforeAsmAstChanger(this, compilerOptions) + val fixer = BeforeAsmAstChanger(this, compilerOptions, errors) fixer.visit(this) while (errors.noErrors() && fixer.applyModifications() > 0) { fixer.visit(this) @@ -155,11 +155,29 @@ internal fun IdentifierReference.isSubroutineParameter(program: Program): Boolea return false } -internal fun Subroutine.hasRtsInAsm(): Boolean { - return statements +internal fun Subroutine.hasRtsInAsm(checkOnlyLastInstruction: Boolean): Boolean { + val asms = statements .asSequence() .filterIsInstance() - .any { it.hasReturnOrRts() } + if(checkOnlyLastInstruction) { + val lastAsm = asms.lastOrNull() ?: return false + val lastLine = lastAsm.assembly.lineSequence().map { it.trim() }.last { + it.isNotBlank() && (!it.startsWith(';') || it.contains("!notreached!")) + } + if(lastLine.contains("!notreached!")) + return true + val inlineAsm = InlineAssembly(" $lastLine", lastAsm.isIR, lastAsm.position) + return inlineAsm.hasReturnOrRts() + } else { + val allAsms = asms.toList() + val lastAsm = allAsms.lastOrNull() ?: return false + val lastLine = lastAsm.assembly.lineSequence().map { it.trim() }.last { + it.isNotBlank() && (!it.startsWith(';') || it.contains("!notreached!")) + } + if(lastLine.contains("!notreached!")) + return true + return allAsms.any { it.hasReturnOrRts() } + } } internal fun IdentifierReference.checkFunctionOrLabelExists(program: Program, statement: Statement, errors: IErrorReporter): Statement? { diff --git a/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt b/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt index 6bc795d83..215f34fea 100644 --- a/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt +++ b/compiler/src/prog8/compiler/astprocessing/BeforeAsmAstChanger.kt @@ -11,7 +11,7 @@ import prog8.ast.walk.IAstModification import prog8.code.core.* import prog8.code.target.VMTarget -internal class BeforeAsmAstChanger(val program: Program, private val options: CompilationOptions) : AstWalker() { +internal class BeforeAsmAstChanger(val program: Program, private val options: CompilationOptions, private val errors: IErrorReporter) : AstWalker() { override fun before(breakStmt: Break, parent: Node): Iterable { throw InternalCompilerException("break should have been replaced by goto $breakStmt") @@ -81,13 +81,22 @@ internal class BeforeAsmAstChanger(val program: Program, private val options: Co } if (!subroutine.inline) { - if (subroutine.isAsmSubroutine && subroutine.asmAddress==null && !subroutine.hasRtsInAsm()) { - // make sure the NOT INLINED asm subroutine actually has a rts at the end - // (non-asm routines get a Return statement as needed, above) - mods += if(options.compTarget.name==VMTarget.NAME) - IAstModification.InsertLast(InlineAssembly(" return\n", true, Position.DUMMY), subroutine) - else - IAstModification.InsertLast(InlineAssembly(" rts\n", false, Position.DUMMY), subroutine) + if (subroutine.isAsmSubroutine && subroutine.asmAddress==null) { + if(!options.addMissingRts && !subroutine.hasRtsInAsm(true)) { + errors.err("asmsub seems to never return as it doesn't end with RTS/JMP/branch. If this is intended, add a '; !notreached!' comment at the end, or use -addmissingrts", subroutine.position) + } + else if (!subroutine.hasRtsInAsm(false)) { + // make sure the NOT INLINED asm subroutine actually has a rts at the end + // (non-asm routines get a Return statement as needed, above) + if(options.addMissingRts) { + errors.info("added missing RTS to asmsub", subroutine.position) + mods += if(options.compTarget.name==VMTarget.NAME) + IAstModification.InsertLast(InlineAssembly(" return\n", true, Position.DUMMY), subroutine) + else + IAstModification.InsertLast(InlineAssembly(" rts\n", false, Position.DUMMY), subroutine) + } else + errors.err("asmsub seems to never return as it doesn't end with RTS/JMP/branch. If this is intended, add a '; !notreached!' comment at the end, or use -addmissingrts", subroutine.position) + } } } diff --git a/compiler/test/TestCompilerOnExamples.kt b/compiler/test/TestCompilerOnExamples.kt index 3ced4659a..30ed22b75 100644 --- a/compiler/test/TestCompilerOnExamples.kt +++ b/compiler/test/TestCompilerOnExamples.kt @@ -40,6 +40,7 @@ private fun compileTheThing(filepath: Path, optimize: Boolean, target: ICompilat slabsGolden = false, compilationTarget = target.name, splitWordArrays = false, + addMissingRts = false, breakpointCpuInstruction = null, printAst1 = false, printAst2 = false, diff --git a/compiler/test/TestCompilerOptionLibdirs.kt b/compiler/test/TestCompilerOptionLibdirs.kt index f0961cf7c..5d6ff30b7 100644 --- a/compiler/test/TestCompilerOptionLibdirs.kt +++ b/compiler/test/TestCompilerOptionLibdirs.kt @@ -38,6 +38,7 @@ class TestCompilerOptionSourcedirs: FunSpec({ slabsGolden = false, compilationTarget = Cx16Target.NAME, splitWordArrays = false, + addMissingRts = false, breakpointCpuInstruction = null, printAst1 = false, printAst2 = false, diff --git a/compiler/test/TestOptimization.kt b/compiler/test/TestOptimization.kt index e2e5b7eeb..4a80cb5ed 100644 --- a/compiler/test/TestOptimization.kt +++ b/compiler/test/TestOptimization.kt @@ -56,6 +56,7 @@ main { asmsub baz() { %asm{{ jsr p8s_foo + jmp blah }} } asmsub bar() { diff --git a/compiler/test/TestSubroutines.kt b/compiler/test/TestSubroutines.kt index 17d16dc8a..dd3d6633f 100644 --- a/compiler/test/TestSubroutines.kt +++ b/compiler/test/TestSubroutines.kt @@ -104,6 +104,9 @@ class TestSubroutines: FunSpec({ } asmsub asmfunc(str thing @AY) { + %asm {{ + rts + }} } sub func(str thing) { @@ -124,7 +127,7 @@ class TestSubroutines: FunSpec({ asmfunc.isAsmSubroutine shouldBe true asmfunc.statements.single() shouldBe instanceOf() (asmfunc.statements.single() as InlineAssembly).assembly.trim() shouldBe "rts" - asmfunc.hasRtsInAsm() shouldBe true + asmfunc.hasRtsInAsm(false) shouldBe true func.isAsmSubroutine shouldBe false withClue("str param should have been changed to uword") { asmfunc.parameters.single().type shouldBe DataType.UWORD diff --git a/compiler/test/codegeneration/TestVariousCodeGen.kt b/compiler/test/codegeneration/TestVariousCodeGen.kt index 22b6c5c44..01aad8fcf 100644 --- a/compiler/test/codegeneration/TestVariousCodeGen.kt +++ b/compiler/test/codegeneration/TestVariousCodeGen.kt @@ -1,6 +1,7 @@ package prog8tests.codegeneration import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.collections.shouldContain import io.kotest.matchers.ints.shouldBeGreaterThan import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe @@ -466,4 +467,52 @@ main { }""" compileText(Cx16Target(), false, src, writeAssembly = true) shouldNotBe null } + + test("missing rts in asmsub") { + val src=""" +main { + sub start() { + test() + test2() + } + + asmsub test() { + %asm {{ + nop + nop + }} + } + + inline asmsub test2() { + %asm {{ + nop + nop + }} + } +}""" + + val errors = ErrorReporterForTests() + compileText(C64Target(), false, src, writeAssembly = true, errors = errors) shouldBe null + errors.errors.size shouldBe 1 + errors.errors[0] shouldContain "asmsub seems to never return" + } + + test("missing rts in asmsub suppressed") { + val src=""" +main { + sub start() { + test() + } + + asmsub test() { + %asm {{ + nop + nop + ; !notreached! + }} + } +}""" + + compileText(C64Target(), false, src, writeAssembly = true) shouldNotBe null + } }) \ No newline at end of file diff --git a/compiler/test/helpers/compileXyz.kt b/compiler/test/helpers/compileXyz.kt index 28ae0e11d..3cb2abfdd 100644 --- a/compiler/test/helpers/compileXyz.kt +++ b/compiler/test/helpers/compileXyz.kt @@ -40,6 +40,7 @@ internal fun compileFile( outputDir = outputDir, errors = errors ?: ErrorReporterForTests(), splitWordArrays = false, + addMissingRts = false, breakpointCpuInstruction = null, printAst1 = false, printAst2 = false, diff --git a/docs/source/compiling.rst b/docs/source/compiling.rst index 467d3f773..e78f92b34 100644 --- a/docs/source/compiling.rst +++ b/docs/source/compiling.rst @@ -182,6 +182,12 @@ One or more .p8 module files interpret certain instructions differently and produce unexpected opcodes (like LDA X getting turned into TXA, or not, depending on the symbol 'x' being defined in your own assembly code or not) +``-addmissingrts`` + Enables old compiler behavior that silently adds RTS to asmsubs that don't have one. + This was done to fix asmsubs so they return properly to the caller instead of crashing the program. + However the new compiler behavior is to not silently modify the code anymore and instead give an error message + that tells you how to fix the problem. This option may go away in future version. + ``-quietasm`` Don't print assembler output results. diff --git a/docs/source/todo.rst b/docs/source/todo.rst index a2f729577..552ea6369 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,6 +1,8 @@ TODO ==== +- check benchmark score vs previous version + Improve register load order in subroutine call args assignments: in certain situations, the "wrong" order of evaluation of function call arguments is done which results in overwriting registers that already got their value, which requires a lot of stack juggling (especially on plain 6502 cpu!) @@ -11,7 +13,6 @@ Future Things and Ideas ^^^^^^^^^^^^^^^^^^^^^^^ - Improve the SublimeText syntax file for prog8, you can also install this for 'bat': https://github.com/sharkdp/bat?tab=readme-ov-file#adding-new-syntaxes--language-definitions - Can we support signed % (remainder) somehow? -- Don't add "random" rts to %asm blocks but instead give a warning about it? (but this breaks existing behavior that others already depend on... command line switch? block directive?) - IR: implement missing operators in AssignmentGen (array shifts etc) - instead of copy-pasting inline asmsubs, make them into a 64tass macro and use that instead. that will allow them to be reused from custom user written assembly code as well. diff --git a/examples/cx16/pcmaudio/stream-simple-aflow.p8 b/examples/cx16/pcmaudio/stream-simple-aflow.p8 index 7afabd203..214be3a25 100644 --- a/examples/cx16/pcmaudio/stream-simple-aflow.p8 +++ b/examples/cx16/pcmaudio/stream-simple-aflow.p8 @@ -71,7 +71,7 @@ interrupts { ; no other irqs in this example. } - asmsub wait() { + inline asmsub wait() { %asm {{ wai }} diff --git a/examples/cx16/pcmaudio/stream-wav.p8 b/examples/cx16/pcmaudio/stream-wav.p8 index 9f7c1a10b..e0a0d0bcd 100644 --- a/examples/cx16/pcmaudio/stream-wav.p8 +++ b/examples/cx16/pcmaudio/stream-wav.p8 @@ -161,7 +161,7 @@ interrupts { bool aflow - asmsub wait() { + inline asmsub wait() { %asm {{ wai }} @@ -285,6 +285,7 @@ _lp2 lda $ffff,y inc _lp2+2 dex bne _loop + ; !notreached! }} ; original prog8 code: ; uword @requirezp ptr = main.start.buffer diff --git a/examples/cx16/pcmaudio/vumeter.p8 b/examples/cx16/pcmaudio/vumeter.p8 index 8ac011ba1..c32e0e1b8 100644 --- a/examples/cx16/pcmaudio/vumeter.p8 +++ b/examples/cx16/pcmaudio/vumeter.p8 @@ -327,7 +327,7 @@ interrupts { bool aflow bool vsync - asmsub wait() { + inline asmsub wait() { %asm {{ wai }} diff --git a/examples/test.p8 b/examples/test.p8 index 531638b6a..3f2d99924 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -4,46 +4,21 @@ main { sub start() { - str @alignword @shared name1 = "abc123456789" - str @alignpage @shared name2 = "def123456789" - str @alignword @shared @nozp name3 = "ghi123456789" - str @alignword @shared @nozp name4 = "jkl123456789" - ubyte[9] @alignword @shared array1 - ubyte[9] @alignword @shared array2 - ubyte[9] @alignpage @shared array3 - ubyte[9] @alignword @shared array4 - ubyte[9] @alignword @shared array5 - ubyte[9] @alignword @shared array6 - ubyte[9] @alignword @shared array7 - ubyte[9] @alignword @shared array8 - ubyte[] @alignword @shared array9 = [1,2,3] - ubyte[] @alignword @shared array10 = [1,2,3] - ubyte[] @alignpage @shared array11 = [1,2,3] - ubyte[] @alignpage @shared array12 = [1,2,3] - ubyte[] @alignword @shared array13 = [1,2,3] - ubyte[] @alignword @shared array14 = [1,2,3] - ubyte[] @alignpage @shared array15 = [1,2,3] - ubyte[] @alignpage @shared array16 = [1,2,3] - uword[3] @alignword @split @shared array17 - uword[] @alignword @split @shared array18 = [1111,2222,3333] + derp() + derp2() + } - array9[2]++ - array10[2]++ - array11[2]++ - array12[2]++ - array13[2]++ - array14[2]++ - array15[2]++ - array16[2]++ - array17[2]++ - array18[2]++ - name1[2]++ - name2[2]++ - name3[2]++ - name4[2]++ + asmsub derp() { + %asm {{ + nop + nop + }} + } - %align 2 - %align 3 - %align 1000 + inline asmsub derp2() { + %asm {{ + nop + nop + }} } }