From 71fd98e39e1e701c75ceef110b9dc8bbd2de2bdc Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Wed, 7 Oct 2020 00:06:42 +0200 Subject: [PATCH] allow asmsub routines with multiple return values to be called (special case for return values in status register) --- .../prog8/ast/expressions/AstExpressions.kt | 6 +++ .../src/prog8/ast/processing/AstChecker.kt | 52 +++++++++++++------ .../ast/processing/VerifyFunctionArgTypes.kt | 16 ++++-- .../src/prog8/ast/statements/AstStatements.kt | 2 +- compiler/src/prog8/compiler/Main.kt | 4 +- .../compiler/target/c64/codegen/AsmGen.kt | 11 ++-- .../target/c64/codegen/ExpressionsAsmGen.kt | 3 +- .../target/c64/codegen/FunctionCallAsmGen.kt | 8 ++- .../c64/codegen/assignment/AsmAssignment.kt | 19 ++++++- .../codegen/assignment/AssignmentAsmGen.kt | 30 ++++++----- docs/source/syntaxreference.rst | 15 ++++-- examples/test.p8 | 15 ++++-- 12 files changed, 130 insertions(+), 51 deletions(-) diff --git a/compiler/src/prog8/ast/expressions/AstExpressions.kt b/compiler/src/prog8/ast/expressions/AstExpressions.kt index a94697e6b..9a5ab1bbd 100644 --- a/compiler/src/prog8/ast/expressions/AstExpressions.kt +++ b/compiler/src/prog8/ast/expressions/AstExpressions.kt @@ -872,6 +872,12 @@ class FunctionCall(override var target: IdentifierReference, return InferredTypes.void() // no return value if(stmt.returntypes.size==1) return InferredTypes.knownFor(stmt.returntypes[0]) + + // multiple return values. Can occur for asmsub routines. If there is exactly one register return value, take that. + val numRegisterReturns = stmt.asmReturnvaluesRegisters.count { it.registerOrPair!=null } + if(numRegisterReturns==1) + return InferredTypes.InferredType.known(DataType.UBYTE) + return InferredTypes.unknown() // has multiple return types... so not a single resulting datatype possible } else -> return InferredTypes.unknown() diff --git a/compiler/src/prog8/ast/processing/AstChecker.kt b/compiler/src/prog8/ast/processing/AstChecker.kt index 19a2f05b2..95baef504 100644 --- a/compiler/src/prog8/ast/processing/AstChecker.kt +++ b/compiler/src/prog8/ast/processing/AstChecker.kt @@ -346,17 +346,15 @@ internal class AstChecker(private val program: Program, } override fun visit(assignment: Assignment) { - // assigning from a functioncall COULD return multiple values (from an asm subroutine) if(assignment.value is FunctionCall) { val stmt = (assignment.value as FunctionCall).target.targetStatement(program.namespace) - if (stmt is Subroutine && stmt.isAsmSubroutine) { - if(stmt.returntypes.size>1) - errors.err("It's not possible to store the multiple results of this asmsub call; you should use a small block of custom inline assembly for this.", assignment.value.position) - else { - val idt = assignment.target.inferType(program, assignment) - if(!idt.isKnown || stmt.returntypes.single()!=idt.typeOrElse(DataType.BYTE)) { - errors.err("return type mismatch", assignment.value.position) - } + if (stmt is Subroutine) { + val idt = assignment.target.inferType(program, assignment) + if(!idt.isKnown) { + errors.err("return type mismatch", assignment.value.position) + } + if(stmt.returntypes.size <= 1 && stmt.returntypes.single()!=idt.typeOrElse(DataType.BYTE)) { + errors.err("return type mismatch", assignment.value.position) } } } @@ -386,7 +384,8 @@ internal class AstChecker(private val program: Program, } val targetDt = assignment.target.inferType(program, assignment) - if(assignment.value.inferType(program) != targetDt) { + val valueDt = assignment.value.inferType(program) + if(valueDt.isKnown && valueDt != targetDt) { if(targetDt.typeOrElse(DataType.STRUCT) in IterableDatatypes) errors.err("cannot assign value to string or array", assignment.value.position) else @@ -446,12 +445,8 @@ internal class AstChecker(private val program: Program, checkValueTypeAndRange(targetDatatype.typeOrElse(DataType.BYTE), constVal) } else { val sourceDatatype = assignment.value.inferType(program) - if (!sourceDatatype.isKnown) { - if (assignment.value is FunctionCall) { - val targetStmt = (assignment.value as FunctionCall).target.targetStatement(program.namespace) - if (targetStmt != null) - errors.err("function call doesn't return a suitable value to use in assignment", assignment.value.position) - } else + if (sourceDatatype.isUnknown) { + if (assignment.value !is FunctionCall) errors.err("assignment value is invalid or has no proper datatype", assignment.value.position) } else { checkAssignmentCompatible(targetDatatype.typeOrElse(DataType.BYTE), assignTarget, @@ -791,6 +786,8 @@ internal class AstChecker(private val program: Program, } override fun visit(expr: BinaryExpression) { + super.visit(expr) + val leftIDt = expr.left.inferType(program) val rightIDt = expr.right.inferType(program) if(!leftIDt.isKnown || !rightIDt.isKnown) @@ -836,7 +833,6 @@ internal class AstChecker(private val program: Program, errors.err("right operand is not numeric", expr.right.position) if(leftDt!=rightDt) errors.err("left and right operands aren't the same type", expr.left.position) - super.visit(expr) } override fun visit(typecast: TypecastExpression) { @@ -898,6 +894,28 @@ internal class AstChecker(private val program: Program, if(error!=null) errors.err(error, functionCall.position) + // check the functions that return multiple returnvalues. + val stmt = functionCall.target.targetStatement(program.namespace) + if (stmt is Subroutine) { + if (stmt.returntypes.size > 1) { + // Currently, it's only possible to handle ONE (or zero) return values from a subroutine. + // asmsub routines can have multiple return values, for instance in 2 different registers. + // It's not (yet) possible to handle these multiple return values because assignments + // are only to a single unique target at the same time. + // EXCEPTION: + // if the asmsub returns multiple values and one of them is via a status register bit, + // it *is* possible to handle them by just actually assigning the register value and + // dealing with the status bit as just being that, the status bit after the call. + val (returnRegisters, returnStatusflags) = stmt.asmReturnvaluesRegisters.partition { rr -> rr.registerOrPair != null } + if (returnRegisters.isEmpty() || returnRegisters.size == 1) { + if (returnStatusflags.any()) + errors.warn("this asmsub also has one or more return 'values' in one of the status flags", functionCall.position) + } else { + errors.err("It's not possible to store the multiple result values of this asmsub call; you should use a small block of custom inline assembly for this.", functionCall.position) + } + } + } + super.visit(functionCall) } diff --git a/compiler/src/prog8/ast/processing/VerifyFunctionArgTypes.kt b/compiler/src/prog8/ast/processing/VerifyFunctionArgTypes.kt index 3baf8d667..100402e83 100644 --- a/compiler/src/prog8/ast/processing/VerifyFunctionArgTypes.kt +++ b/compiler/src/prog8/ast/processing/VerifyFunctionArgTypes.kt @@ -4,10 +4,9 @@ import prog8.ast.IFunctionCall import prog8.ast.INameScope import prog8.ast.Program import prog8.ast.base.DataType +import prog8.ast.expressions.Expression import prog8.ast.expressions.FunctionCall -import prog8.ast.statements.BuiltinFunctionStatementPlaceholder -import prog8.ast.statements.FunctionCallStatement -import prog8.ast.statements.Subroutine +import prog8.ast.statements.* import prog8.compiler.CompilerException import prog8.functions.BuiltinFunctions @@ -43,7 +42,6 @@ class VerifyFunctionArgTypes(val program: Program) : IAstVisitor { val argtypes = call.args.map { it.inferType(program).typeOrElse(DataType.STRUCT) } val target = call.target.targetStatement(scope) if (target is Subroutine) { - // asmsub types are not checked specifically at this time if(call.args.size != target.parameters.size) return "invalid number of arguments" val paramtypes = target.parameters.map { it.type } @@ -53,6 +51,16 @@ class VerifyFunctionArgTypes(val program: Program) : IAstVisitor { val expected = paramtypes[mismatch].toString() return "argument ${mismatch + 1} type mismatch, was: $actual expected: $expected" } + if(target.isAsmSubroutine) { + if(target.asmReturnvaluesRegisters.size>1) { + // multiple return values will NOT work inside an expression. + // they MIGHT work in a regular assignment or just a function call statement. + val parent = if(call is Statement) call.parent else if(call is Expression) call.parent else null + if(call !is FunctionCallStatement && parent !is Assignment && parent !is VarDecl) { + return "can't use multiple return values here" + } + } + } } else if (target is BuiltinFunctionStatementPlaceholder) { val func = BuiltinFunctions.getValue(target.name) diff --git a/compiler/src/prog8/ast/statements/AstStatements.kt b/compiler/src/prog8/ast/statements/AstStatements.kt index 59cc84084..660f231ea 100644 --- a/compiler/src/prog8/ast/statements/AstStatements.kt +++ b/compiler/src/prog8/ast/statements/AstStatements.kt @@ -416,7 +416,7 @@ data class AssignTarget(var identifier: IdentifierReference?, } } - fun inferType(program: Program, stmt: Statement): InferredTypes.InferredType { + fun inferType(program: Program, stmt: Statement): InferredTypes.InferredType { // TODO why does this have the extra 'stmt' scope parameter??? if (identifier != null) { val symbol = program.namespace.lookup(identifier!!.nameInSource, stmt) ?: return InferredTypes.unknown() if (symbol is VarDecl) return InferredTypes.knownFor(symbol.datatype) diff --git a/compiler/src/prog8/compiler/Main.kt b/compiler/src/prog8/compiler/Main.kt index 58fa2703a..08a4e31b4 100644 --- a/compiler/src/prog8/compiler/Main.kt +++ b/compiler/src/prog8/compiler/Main.kt @@ -56,7 +56,7 @@ fun compileProgram(filepath: Path, optimizeAst(programAst, errors) postprocessAst(programAst, errors, compilationOptions) - printAst(programAst) // TODO + // printAst(programAst) if(writeAssembly) programName = writeAssembly(programAst, errors, outputDir, optimize, compilationOptions) @@ -219,7 +219,7 @@ private fun writeAssembly(programAst: Program, errors: ErrorReporter, outputDir: programAst.processAstBeforeAsmGeneration(errors) errors.handle() - printAst(programAst) // TODO + // printAst(programAst) CompilationTarget.instance.machine.initializeZeropage(compilerOptions) val assembly = CompilationTarget.instance.asmGenerator( diff --git a/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt b/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt index f23986eed..de6cd981d 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen/AsmGen.kt @@ -588,9 +588,10 @@ internal class AsmGen(private val program: Program, if (builtinFunc != null) { builtinFunctionsAsmGen.translateFunctioncallStatement(stmt, builtinFunc) } else { - functioncallAsmGen.translateFunctionCall(stmt) - // discard any results from the stack: val sub = stmt.target.targetSubroutine(program.namespace)!! + val preserveStatusRegisterAfterCall = sub.asmReturnvaluesRegisters.any {it.statusflag!=null} + functioncallAsmGen.translateFunctionCall(stmt, preserveStatusRegisterAfterCall) + // discard any results from the stack: val returns = sub.returntypes.zip(sub.asmReturnvaluesRegisters) for ((t, reg) in returns) { if (reg.stack) { @@ -598,6 +599,8 @@ internal class AsmGen(private val program: Program, else if (t == DataType.FLOAT) out(" inx | inx | inx") } } + if(preserveStatusRegisterAfterCall) + out(" plp\t; restore status flags from call") } } is Assignment -> assignmentAsmGen.translate(stmt) @@ -763,8 +766,8 @@ internal class AsmGen(private val program: Program, internal fun translateFunctioncallExpression(functionCall: FunctionCall, signature: FSignature) = builtinFunctionsAsmGen.translateFunctioncallExpression(functionCall, signature) - internal fun translateFunctionCall(functionCall: FunctionCall) = - functioncallAsmGen.translateFunctionCall(functionCall) + internal fun translateFunctionCall(functionCall: FunctionCall, preserveStatusRegisterAfterCall: Boolean) = + functioncallAsmGen.translateFunctionCall(functionCall, preserveStatusRegisterAfterCall) 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 31b147a42..5d159bcbf 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen/ExpressionsAsmGen.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen/ExpressionsAsmGen.kt @@ -949,7 +949,8 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge asmgen.translateFunctioncallExpression(expression, builtinFunc) } else { val sub = expression.target.targetSubroutine(program.namespace)!! - asmgen.translateFunctionCall(expression) + val preserveStatusRegisterAfterCall = sub.asmReturnvaluesRegisters.any {it.statusflag!=null} + asmgen.translateFunctionCall(expression, preserveStatusRegisterAfterCall) val returns = sub.returntypes.zip(sub.asmReturnvaluesRegisters) for ((_, reg) in returns) { if (!reg.stack) { diff --git a/compiler/src/prog8/compiler/target/c64/codegen/FunctionCallAsmGen.kt b/compiler/src/prog8/compiler/target/c64/codegen/FunctionCallAsmGen.kt index 836edf320..af0fb3335 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen/FunctionCallAsmGen.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen/FunctionCallAsmGen.kt @@ -13,7 +13,7 @@ import prog8.compiler.target.c64.codegen.assignment.* internal class FunctionCallAsmGen(private val program: Program, private val asmgen: AsmGen) { - internal fun translateFunctionCall(stmt: IFunctionCall) { + internal fun translateFunctionCall(stmt: IFunctionCall, preserveStatusRegisterAfterCall: Boolean) { // output the code to setup the parameters and perform the actual call // does NOT output the code to deal with the result values! val sub = stmt.target.targetSubroutine(program.namespace) ?: throw AssemblyError("undefined subroutine ${stmt.target}") @@ -57,6 +57,12 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg } asmgen.out(" jsr $subName") + if(preserveStatusRegisterAfterCall) { + asmgen.out(" php\t; save status flags from call") + // note: the containing statement (such as the FunctionCallStatement or the Assignment or the Expression) + // must take care of popping this value again at the end! + } + if(saveX) asmgen.restoreRegister(CpuRegister.X) } diff --git a/compiler/src/prog8/compiler/target/c64/codegen/assignment/AsmAssignment.kt b/compiler/src/prog8/compiler/target/c64/codegen/assignment/AsmAssignment.kt index 859813d6e..6e6ad88ab 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen/assignment/AsmAssignment.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen/assignment/AsmAssignment.kt @@ -6,6 +6,7 @@ import prog8.ast.expressions.* import prog8.ast.statements.AssignTarget import prog8.ast.statements.Assignment import prog8.ast.statements.DirectMemoryWrite +import prog8.ast.statements.Subroutine import prog8.compiler.AssemblyError import prog8.compiler.target.c64.codegen.AsmGen @@ -118,8 +119,23 @@ internal class AsmAssignSource(val kind: SourceStorageKind, AsmAssignSource(SourceStorageKind.ARRAY, program, dt, array = value) } else -> { + if(value is FunctionCall) { + // functioncall. + val asmSub = value.target.targetStatement(program.namespace) + if(asmSub is Subroutine && asmSub.isAsmSubroutine) { + when (asmSub.asmReturnvaluesRegisters.count { rr -> rr.registerOrPair!=null }) { + 0 -> throw AssemblyError("can't translate zero return values in assignment") + 1 -> { + // assignment generation itself must make sure the status register is correct after the subroutine call, if status register is involved! + return AsmAssignSource(SourceStorageKind.EXPRESSION, program, DataType.UBYTE, expression = value) + } + else -> throw AssemblyError("can't translate multiple return values in assignment") + } + } + } + val dt = value.inferType(program).typeOrElse(DataType.STRUCT) - AsmAssignSource(SourceStorageKind.EXPRESSION, program, dt, expression = value) + return AsmAssignSource(SourceStorageKind.EXPRESSION, program, dt, expression = value) } } } @@ -160,6 +176,7 @@ internal class AsmAssignment(val source: AsmAssignSource, init { if(target.register !in setOf(RegisterOrPair.XY, RegisterOrPair.AX, RegisterOrPair.AY)) + require(source.datatype != DataType.STRUCT) { "must not be placeholder datatype" } require(source.datatype.memorySize() == target.datatype.memorySize()) { "source and target datatype must be same storage class" } } } 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 5abdf852c..9dd64d183 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen/assignment/AssignmentAsmGen.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen/assignment/AssignmentAsmGen.kt @@ -123,20 +123,22 @@ internal class AssignmentAsmGen(private val program: Program, private val asmgen is ArrayIndexedExpression -> throw AssemblyError("source kind should have been array") is DirectMemoryRead -> throw AssemblyError("source kind should have been memory") is TypecastExpression -> assignTypeCastedValue(assign.target, value.type, value.expression, assign) -// is FunctionCall -> { -// if (assign.target.kind == TargetStorageKind.STACK) { -// asmgen.translateExpression(value) -// assignStackValue(assign.target) -// } else { -// val functionName = value.target.nameInSource.last() -// val builtinFunc = BuiltinFunctions[functionName] -// if (builtinFunc != null) { -// println("!!!!BUILTIN-FUNCCALL target=${assign.target.kind} $value") // TODO optimize certain functions? -// } -// asmgen.translateExpression(value) -// assignStackValue(assign.target) -// } -// } + is FunctionCall -> { + if(value.target.targetSubroutine(program.namespace)?.isAsmSubroutine==true) { + // TODO handle asmsub functioncalls specifically, without shoving stuff on the estack + asmgen.translateExpression(value) + assignStackValue(assign.target) + val sub = value.target.targetSubroutine(program.namespace) + if(sub!=null && sub.asmReturnvaluesRegisters.any { it.statusflag != null }) { + // the expression translation will have generated a 'php' instruction earlier. + asmgen.out(" plp\t; restore status flags from call") + } + } else { + // regular subroutine, return values are (for now) always done via the stack... TODO optimize this + asmgen.translateExpression(value) + assignStackValue(assign.target) + } + } else -> { // everything else just evaluate via the stack. asmgen.translateExpression(value) diff --git a/docs/source/syntaxreference.rst b/docs/source/syntaxreference.rst index 28fdc6237..68a48c220 100644 --- a/docs/source/syntaxreference.rst +++ b/docs/source/syntaxreference.rst @@ -503,6 +503,8 @@ takes no parameters. If the subroutine returns a value, usually you assign it t If you're not interested in the return value, prefix the function call with the ``void`` keyword. Otherwise the compiler will warn you about discarding the result of the call. +Multiple return values +^^^^^^^^^^^^^^^^^^^^^^ Normal subroutines can only return zero or one return values. However, the special ``asmsub`` routines (implemented in assembly code) or ``romsub`` routines (referencing a routine in kernel ROM) can return more than one return value. @@ -510,9 +512,16 @@ For example a status in the carry bit and a number in A, or a 16-bit value in A/ It is not possible to process the results of a call to these kind of routines directly from the language, because only single value assignments are possible. You can still call the subroutine and not store the results. -But if you want to do something with the values it returns, you'll have to write -a small block of custom inline assembly that does the call and stores the values -appropriately. Don't forget to save/restore the registers if required. + +**There is an exception:** if there's just one return value in a register, and one or more others that are returned +as bits in the status register (such as the Carry bit), the compiler allows you to call the subroutine. +It will then store the result value in a variable if required, and *keep the status register untouched +after the call* so you can use a conditional branch statement for that. +Note that this makes no sense inside an expression, so the compiler will still give an error for that. + +If there really are multiple return values (other than a combined 16 bit return value in 2 registers), +you'll have to write a small block of custom inline assembly that does the call and stores the values +appropriately. Don't forget to save/restore any registers that are modified. Subroutine definitions diff --git a/examples/test.p8 b/examples/test.p8 index 967821ca3..891937b54 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -8,11 +8,20 @@ main { str planet_name = "12345678" sub start() { - ; TODO make this work, with a warning about Pc: + c64.OPEN() ; works: function call droppign the value but preserving the statusregister + if_cs + return + ubyte status status = c64.OPEN() ; open 1,8,0,"$" - ; TODO make this work as well, with the same warning: - ubyte status2 = c64.OPEN() ; open 1,8,0,"$" + if_cs + return + + +; ; TODO make this work as well, with the same warning: +; ubyte status2 = c64.OPEN() ; open 1,8,0,"$" + if_cs + return