From 3550e1214c80ff82f256df0804c7fc526ca662f3 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Tue, 5 Jan 2021 01:41:32 +0100 Subject: [PATCH] fix invalid handling of X register functioncall result value --- .../src/prog8/ast/processing/AstChecker.kt | 2 - .../compiler/target/c64/codegen/AsmGen.kt | 7 +++ .../target/c64/codegen/ExpressionsAsmGen.kt | 45 +++++++------------ .../target/c64/codegen/FunctionCallAsmGen.kt | 45 ++++++++++++------- .../codegen/assignment/AssignmentAsmGen.kt | 5 +++ examples/test.p8 | 16 ++++--- 6 files changed, 65 insertions(+), 55 deletions(-) diff --git a/compiler/src/prog8/ast/processing/AstChecker.kt b/compiler/src/prog8/ast/processing/AstChecker.kt index c3a240507..fdc095f72 100644 --- a/compiler/src/prog8/ast/processing/AstChecker.kt +++ b/compiler/src/prog8/ast/processing/AstChecker.kt @@ -1029,8 +1029,6 @@ internal class AstChecker(private val program: Program, } } } else if(target is Subroutine) { - if(target.regXasResult()) - errors.warn("subroutine call return value in X register is discarded and replaced by 0", position) if(target.isAsmSubroutine) { for (arg in args.zip(target.parameters)) { val argIDt = arg.first.inferType(program) diff --git a/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt b/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt index a2f2e84ba..a72ae0415 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt @@ -1,5 +1,6 @@ package prog8.compiler.target.c64.codegen +import prog8.ast.IFunctionCall import prog8.ast.INameScope import prog8.ast.Node import prog8.ast.Program @@ -792,6 +793,12 @@ internal class AsmGen(private val program: Program, internal fun translateFunctionCall(functionCall: FunctionCall) = functioncallAsmGen.translateFunctionCall(functionCall) + internal fun saveXbeforeCall(functionCall: IFunctionCall) = + functioncallAsmGen.saveXbeforeCall(functionCall) + + internal fun restoreXafterCall(functionCall: IFunctionCall) = + functioncallAsmGen.restoreXafterCall(functionCall) + internal fun translateNormalAssignment(assign: AsmAssignment) = assignmentAsmGen.translateNormalAssignment(assign) diff --git a/compiler/src/prog8/compiler/target/c64/codegen/ExpressionsAsmGen.kt b/compiler/src/prog8/compiler/target/c64/codegen/ExpressionsAsmGen.kt index 13463f590..598f1252f 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen/ExpressionsAsmGen.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen/ExpressionsAsmGen.kt @@ -1324,49 +1324,34 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge +""") } - private fun translateFunctionCallResultOntoStack(expression: FunctionCall) { + private fun translateFunctionCallResultOntoStack(call: FunctionCall) { // only for use in nested expression evaluation - val sub = expression.target.targetStatement(program.namespace) + val sub = call.target.targetStatement(program.namespace) if(sub is BuiltinFunctionStatementPlaceholder) { val builtinFunc = BuiltinFunctions.getValue(sub.name) - asmgen.translateBuiltinFunctionCallExpression(expression, builtinFunc, true) + asmgen.translateBuiltinFunctionCallExpression(call, builtinFunc, true) } else { sub as Subroutine - asmgen.translateFunctionCall(expression) + asmgen.saveXbeforeCall(call) + asmgen.translateFunctionCall(call) + if(sub.regXasResult()) { + // store the return value in X somewhere that we can acces again below + asmgen.out(" stx P8ZP_SCRATCH_REG") + } + asmgen.restoreXafterCall(call) + val returns = sub.returntypes.zip(sub.asmReturnvaluesRegisters) for ((_, reg) in returns) { - // result value in cpu or status registers, put it on the stack + // result value is in cpu or status registers, put it on the stack instead (as we're evaluating an expression tree) if (reg.registerOrPair != null) { when (reg.registerOrPair) { RegisterOrPair.A -> asmgen.out(" sta P8ESTACK_LO,x | dex") RegisterOrPair.Y -> asmgen.out(" tya | sta P8ESTACK_LO,x | dex") RegisterOrPair.AY -> asmgen.out(" sta P8ESTACK_LO,x | tya | sta P8ESTACK_HI,x | dex") - RegisterOrPair.X -> { - // return value in X register has been discarded, just push a zero - if(CompilationTarget.instance.machine.cpu==CpuType.CPU65c02) - asmgen.out(" stz P8ESTACK_LO,x") - else - asmgen.out(" lda #0 | sta P8ESTACK_LO,x") - asmgen.out(" dex") - } - RegisterOrPair.AX -> { - // return value in X register has been discarded, just push a zero in this place - asmgen.out(" sta P8ESTACK_LO,x") - if(CompilationTarget.instance.machine.cpu==CpuType.CPU65c02) - asmgen.out(" stz P8ESTACK_HI,x") - else - asmgen.out(" lda #0 | sta P8ESTACK_HI,x") - asmgen.out(" dex") - } - RegisterOrPair.XY -> { - // return value in X register has been discarded, just push a zero in this place - if(CompilationTarget.instance.machine.cpu==CpuType.CPU65c02) - asmgen.out(" stz P8ESTACK_LO,x") - else - asmgen.out(" lda #0 | sta P8ESTACK_LO,x") - asmgen.out(" tya | sta P8ESTACK_HI,x | dex") - } + RegisterOrPair.X -> asmgen.out(" lda P8ZP_SCRATCH_REG | sta P8ESTACK_LO,x | dex") + RegisterOrPair.AX -> asmgen.out(" sta P8ESTACK_LO,x | lda P8ZP_SCRATCH_REG | sta P8ESTACK_HI,x | dex") + RegisterOrPair.XY -> asmgen.out(" tya | sta P8ESTACK_HI,x | lda P8ZP_SCRATCH_REG | sta P8ESTACK_LO,x | dex") RegisterOrPair.FAC1 -> asmgen.out(" jsr floats.push_fac1") RegisterOrPair.FAC2 -> asmgen.out(" jsr floats.push_fac2") RegisterOrPair.R0, diff --git a/compiler/src/prog8/compiler/target/c64/codegen/FunctionCallAsmGen.kt b/compiler/src/prog8/compiler/target/c64/codegen/FunctionCallAsmGen.kt index 46fec71ec..36eda3ea4 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen/FunctionCallAsmGen.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen/FunctionCallAsmGen.kt @@ -7,7 +7,6 @@ import prog8.ast.base.* import prog8.ast.expressions.* import prog8.ast.statements.* import prog8.compiler.AssemblyError -import prog8.compiler.CompilationOptions import prog8.compiler.target.CompilationTarget import prog8.compiler.target.CpuType import prog8.compiler.target.c64.codegen.assignment.* @@ -16,25 +15,44 @@ import prog8.compiler.target.c64.codegen.assignment.* internal class FunctionCallAsmGen(private val program: Program, private val asmgen: AsmGen) { internal fun translateFunctionCallStatement(stmt: IFunctionCall) { + saveXbeforeCall(stmt) translateFunctionCall(stmt) - // functioncalls no longer return results on the stack, so simply ignore the results in the registers + restoreXafterCall(stmt) + // just ignore any result values from the function call. } - - internal fun translateFunctionCall(stmt: IFunctionCall) { - // output the code to setup the parameters and perform the actual call - // does NOT output the code to deal with the result values! + internal fun saveXbeforeCall(stmt: IFunctionCall) { val sub = stmt.target.targetSubroutine(program.namespace) ?: throw AssemblyError("undefined subroutine ${stmt.target}") - val saveX = sub.shouldSaveX() - val regSaveOnStack = sub.asmAddress==null // rom-routines don't require registers to be saved on stack, normal subroutines do because they can contain nested calls - val (keepAonEntry: Boolean, keepAonReturn: Boolean) = sub.shouldKeepA() - if(saveX) { + if(sub.shouldSaveX()) { + val regSaveOnStack = sub.asmAddress==null // rom-routines don't require registers to be saved on stack, normal subroutines do because they can contain nested calls + val (keepAonEntry: Boolean, keepAonReturn: Boolean) = sub.shouldKeepA() if(regSaveOnStack) asmgen.saveRegisterStack(CpuRegister.X, keepAonEntry) else asmgen.saveRegisterLocal(CpuRegister.X, (stmt as Node).definingSubroutine()!!) } + } + internal fun restoreXafterCall(stmt: IFunctionCall) { + val sub = stmt.target.targetSubroutine(program.namespace) ?: throw AssemblyError("undefined subroutine ${stmt.target}") + if(sub.shouldSaveX()) { + val regSaveOnStack = sub.asmAddress==null // rom-routines don't require registers to be saved on stack, normal subroutines do because they can contain nested calls + val (keepAonEntry: Boolean, keepAonReturn: Boolean) = sub.shouldKeepA() + + if(regSaveOnStack) + asmgen.restoreRegisterStack(CpuRegister.X, keepAonReturn) + else + asmgen.restoreRegisterLocal(CpuRegister.X) + } + } + + internal fun translateFunctionCall(stmt: IFunctionCall) { + // Output only the code to setup the parameters and perform the actual call + // NOTE: does NOT output the code to deal with the result values! + // NOTE: does NOT output code to save/restore the X register for this call! Every caller should deal with this in their own way!! + // (you can use subroutine.shouldSaveX() and saveX()/restoreX() routines as a help for this) + + val sub = stmt.target.targetSubroutine(program.namespace) ?: throw AssemblyError("undefined subroutine ${stmt.target}") val subName = asmgen.asmSymbolName(stmt.target) if(stmt.args.isNotEmpty()) { if(sub.asmParameterRegisters.isEmpty()) { @@ -84,12 +102,7 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg asmgen.out(" jsr $subName") } - if(saveX) { - if(regSaveOnStack) - asmgen.restoreRegisterStack(CpuRegister.X, keepAonReturn) - else - asmgen.restoreRegisterLocal(CpuRegister.X) - } + // remember: dealing with the X register and/or dealing with return values is the responsibility of the caller } private fun registerArgsViaStackEvaluation(stmt: IFunctionCall, sub: Subroutine) { diff --git a/compiler/src/prog8/compiler/target/c64/codegen/assignment/AssignmentAsmGen.kt b/compiler/src/prog8/compiler/target/c64/codegen/assignment/AssignmentAsmGen.kt index 83e3c1bd5..0914c562c 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen/assignment/AssignmentAsmGen.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen/assignment/AssignmentAsmGen.kt @@ -142,7 +142,10 @@ internal class AssignmentAsmGen(private val program: Program, private val asmgen is FunctionCall -> { when (val sub = value.target.targetStatement(program.namespace)) { is Subroutine -> { + asmgen.saveXbeforeCall(value) asmgen.translateFunctionCall(value) + if(!sub.regXasResult()) + asmgen.restoreXafterCall(value) val returnValue = sub.returntypes.zip(sub.asmReturnvaluesRegisters).singleOrNull { it.second.registerOrPair!=null } ?: sub.returntypes.zip(sub.asmReturnvaluesRegisters).single { it.second.statusflag!=null } when (returnValue.first) { @@ -186,6 +189,8 @@ internal class AssignmentAsmGen(private val program: Program, private val asmgen throw AssemblyError("should be just one register byte result value") } } + // we've processed the result value in the X register by now, so it's now safe to restore it: + asmgen.restoreXafterCall(value) } } } diff --git a/examples/test.p8 b/examples/test.p8 index e104cf37e..0a5abde6a 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -6,14 +6,16 @@ main { sub start () { - uword current_time + uword mem + + mem = 5*mem*c64.MEMTOP(0,1) + txt.print_uwhex(mem, 1) + + mem = 5*mem*c64.CHRIN() + txt.print_uwhex(mem,1) + + test_stack.test() - repeat 6 { - current_time = c64.RDTIM16() - txt.print_uw(current_time) - txt.chrout('\n') - sys.wait(30) - } ; found = strfind("irmen de jong", ' ')