From 81f7419f709d71849286fee8fb47df313c9d1df7 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sat, 12 Sep 2020 00:59:57 +0200 Subject: [PATCH] fix X register clobbering in asmfunc call, fixed graphics.plot() --- compiler/build.gradle | 4 +- compiler/res/prog8lib/c64graphics.p8 | 12 ++-- .../compiler/target/c64/codegen/AsmGen.kt | 58 ++++++++++++++----- .../target/c64/codegen/FunctionCallAsmGen.kt | 5 +- examples/mandelbrot-gfx.p8 | 4 +- 5 files changed, 55 insertions(+), 28 deletions(-) diff --git a/compiler/build.gradle b/compiler/build.gradle index a8e517542..5482d0e48 100644 --- a/compiler/build.gradle +++ b/compiler/build.gradle @@ -1,11 +1,11 @@ buildscript { dependencies { - classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:1.4.0" + classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:1.4.10" } } plugins { - // id "org.jetbrains.kotlin.jvm" version "1.4.0" + // id "org.jetbrains.kotlin.jvm" version "1.4.10" id 'application' id 'org.jetbrains.dokka' version "0.9.18" id 'com.github.johnrengelman.shadow' version '5.2.0' diff --git a/compiler/res/prog8lib/c64graphics.p8 b/compiler/res/prog8lib/c64graphics.p8 index 1811aef4b..ab375c559 100644 --- a/compiler/res/prog8lib/c64graphics.p8 +++ b/compiler/res/prog8lib/c64graphics.p8 @@ -181,9 +181,7 @@ graphics { ; @(addr) |= ormask[lsb(px) & 7] ; } - ; TODO fix the use of X (or XY) as parameter so we can actually use this plot() routine - ; calling it with a byte results in a compiler crash, calling it with word results in clobbering X register I think - asmsub plotXXX(uword plotx @XY, ubyte ploty @A) { + asmsub plot(uword plotx @XY, ubyte ploty @A) clobbers (A, X, Y) { %asm {{ stx internal_plotx sty internal_plotx+1 @@ -191,12 +189,14 @@ graphics { }} } + ; for efficiency of internal algorithms here is the internal plot routine + ; that takes the plotx coordinate in a separate variable instead of the XY register pair: + uword internal_plotx ; 0..319 ; separate 'parameter' for internal_plot() - asmsub internal_plot(ubyte ploty @A) { ; internal_plotx is 16 bits 0 to 319... doesn't fit in a register + asmsub internal_plot(ubyte ploty @A) clobbers (A, X, Y) { ; internal_plotx is 16 bits 0 to 319... doesn't fit in a register %asm {{ tay - stx P8ZP_SCRATCH_REG_X lda internal_plotx+1 sta P8ZP_SCRATCH_W2+1 lsr a ; 0 @@ -220,8 +220,6 @@ graphics { lda (P8ZP_SCRATCH_W2),y ora _ormask,x sta (P8ZP_SCRATCH_W2),y - - ldx P8ZP_SCRATCH_REG_X rts _ormask .byte 128, 64, 32, 16, 8, 4, 2, 1 diff --git a/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt b/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt index 988db2be6..c126be98c 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt @@ -552,32 +552,61 @@ internal class AsmGen(private val program: Program, internal fun fixNameSymbols(name: String) = name.replace("<", "prog8_").replace(">", "") // take care of the autogenerated invalid (anon) label names - internal fun saveRegister(register: CpuRegister) { + internal fun saveRegister(register: CpuRegister, forFuncCall: Boolean = false, dontClobberA: Boolean = false) { when(register) { CpuRegister.A -> out(" pha") CpuRegister.X -> { - if(CompilationTarget.instance.machine.cpu == CpuType.CPU65c02) - out(" phx") - else - out(" stx P8ZP_SCRATCH_REG_X") + // TODO get rid of REG_X altogheter! replace with sta _save | txa | pha | lda _save + when { + CompilationTarget.instance.machine.cpu == CpuType.CPU65c02 -> out(" phx") + forFuncCall -> { + if(dontClobberA) + out(" sta P8ZP_SCRATCH_REG | txa | pha | lda P8ZP_SCRATCH_REG") + else + out(" txa | pha") + } + else -> out(" stx P8ZP_SCRATCH_REG_X") + } + } + CpuRegister.Y -> { + when { + CompilationTarget.instance.machine.cpu == CpuType.CPU65c02 -> out(" phy") + dontClobberA -> out(" sta P8ZP_SCRATCH_REG | tya | pha | lda P8ZP_SCRATCH_REG") + else -> out(" tya | pha") + } } - CpuRegister.Y -> out(" tya | pha") } } - internal fun restoreRegister(register: CpuRegister) { + internal fun restoreRegister(register: CpuRegister, forFuncCall: Boolean = false, dontClobberA: Boolean=false) { when(register) { - CpuRegister.A -> out(" pla") - CpuRegister.X -> { - if(CompilationTarget.instance.machine.cpu == CpuType.CPU65c02) - out(" plx") - else - out(" ldx P8ZP_SCRATCH_REG_X") + CpuRegister.A -> { + require(!dontClobberA) + out(" pla") + } + CpuRegister.X -> { + when { + CompilationTarget.instance.machine.cpu == CpuType.CPU65c02 -> out(" plx") + forFuncCall -> { + if(dontClobberA) + out(" sta P8ZP_SCRATCH_REG | pla | tax | lda P8ZP_SCRATCH_REG") + else + out(" pla | tax") + } + else -> out(" ldx P8ZP_SCRATCH_REG_X") + } + } + CpuRegister.Y -> { + when { + CompilationTarget.instance.machine.cpu == CpuType.CPU65c02 -> out(" ply") + dontClobberA -> out(" sta P8ZP_SCRATCH_REG | pla | tay | lda P8ZP_SCRATCH_REG") + else -> out(" pla | tay") + } } - CpuRegister.Y -> out(" pla | tay") } } + internal fun translate(stmt: Statement) { outputSourceLine(stmt) when(stmt) { @@ -796,6 +825,7 @@ internal class AsmGen(private val program: Program, } private fun translate(stmt: IfStatement) { + // TODO don't generate needless jumps/labels when the if or else block is empty expressionsAsmGen.translateExpression(stmt.condition) translateTestStack(stmt.condition.inferType(program).typeOrElse(DataType.STRUCT)) val elseLabel = makeLabel("if_else") diff --git a/compiler/src/prog8/compiler/target/c64/codegen/FunctionCallAsmGen.kt b/compiler/src/prog8/compiler/target/c64/codegen/FunctionCallAsmGen.kt index 13fd48e8e..2227c0fe5 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen/FunctionCallAsmGen.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen/FunctionCallAsmGen.kt @@ -19,7 +19,7 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg val sub = stmt.target.targetSubroutine(program.namespace) ?: throw AssemblyError("undefined subroutine ${stmt.target}") val saveX = CpuRegister.X in sub.asmClobbers || sub.regXasResult() || sub.regXasParam() if(saveX) - asmgen.saveRegister(CpuRegister.X) // we only save X for now (required! is the eval stack pointer), screw A and Y... + asmgen.saveRegister(CpuRegister.X, forFuncCall = true, dontClobberA = false) // we only save X for now (required! is the eval stack pointer), screw A and Y... val subName = asmgen.asmSymbolName(stmt.target) if(stmt.args.isNotEmpty()) { @@ -58,7 +58,7 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg asmgen.out(" jsr $subName") if(saveX) - asmgen.restoreRegister(CpuRegister.X) + asmgen.restoreRegister(CpuRegister.X, forFuncCall = true, dontClobberA = false) } private fun registerArgsViaStackEvaluation(stmt: IFunctionCall, sub: Subroutine) { @@ -120,6 +120,7 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg } if(argForXregister!=null) { + if(argForAregister!=null) asmgen.out(" pha") when(argForXregister.value.second.registerOrPair) { diff --git a/examples/mandelbrot-gfx.p8 b/examples/mandelbrot-gfx.p8 index 9e30880be..4103e4095 100644 --- a/examples/mandelbrot-gfx.p8 +++ b/examples/mandelbrot-gfx.p8 @@ -38,9 +38,7 @@ main { } if iter & 1 - ; TODO fix plot() so we don't have to use separate internal variable - graphics.internal_plotx = pixelx - graphics.internal_plot(pixely) + graphics.plot(pixelx, pixely) ; TODO get rid of typecast in pixelx arg } }