From 54025d2bf5714cd9f564b81103671f95f830d551 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sat, 27 Nov 2021 14:08:34 +0100 Subject: [PATCH] small refactor and spelling fixes --- .../cpu6502/codegen/FunctionCallAsmGen.kt | 139 +++++++++--------- .../codegen/assignment/AsmAssignment.kt | 4 +- .../assignment/AugmentableAssignmentAsmGen.kt | 23 ++- compiler/res/prog8lib/cx16/syslib.p8 | 2 +- docs/source/todo.rst | 3 +- examples/test.p8 | 17 +-- 6 files changed, 91 insertions(+), 97 deletions(-) diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/FunctionCallAsmGen.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/FunctionCallAsmGen.kt index 510738db5..0b9b0668e 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/FunctionCallAsmGen.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/FunctionCallAsmGen.kt @@ -74,88 +74,87 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg // (you can use subroutine.shouldSaveX() and saveX()/restoreX() routines as a help for this) val sub = call.target.targetSubroutine(program) ?: throw AssemblyError("undefined subroutine ${call.target}") + val subAsmName = asmgen.asmSymbolName(call.target) - if(!isExpression && !sub.isAsmSubroutine) { - throw AssemblyError("functioncall statments to non-asmsub should have been replaced by GoSub $call") - } + if(!isExpression && !sub.isAsmSubroutine) + throw AssemblyError("functioncall statements to non-asmsub should have been replaced by GoSub $call") - if(call.args.isNotEmpty()) { - - if(sub.asmParameterRegisters.isEmpty()) { - // via variables - for(arg in sub.parameters.withIndex().zip(call.args)) { - argumentViaVariable(sub, arg.first, arg.second) - } + if(sub.isAsmSubroutine) { + argumentsViaRegisters(sub, call) + if (sub.inline && asmgen.options.optimize) { + // inline the subroutine. + // we do this by copying the subroutine's statements at the call site. + // NOTE: *if* there is a return statement, it will be the only one, and the very last statement of the subroutine + // (this condition has been enforced by an ast check earlier) + asmgen.out(" \t; inlined routine follows: ${sub.name}") + val assembly = sub.statements.single() as InlineAssembly + asmgen.translate(assembly) + asmgen.out(" \t; inlined routine end: ${sub.name}") } else { - require(sub.isAsmSubroutine) - if(sub.parameters.size==1) { - // just a single parameter, no risk of clobbering registers - argumentViaRegister(sub, IndexedValue(0, sub.parameters.single()), call.args[0]) - } else { - - fun isNoClobberRisk(expr: Expression): Boolean { - if(expr is AddressOf || - expr is NumericLiteralValue || - expr is StringLiteralValue || - expr is ArrayLiteralValue || - expr is IdentifierReference) - return true - - if(expr is FunctionCall) { - if(expr.target.nameInSource==listOf("lsb") || expr.target.nameInSource==listOf("msb")) - return isNoClobberRisk(expr.args[0]) - if(expr.target.nameInSource==listOf("mkword")) - return isNoClobberRisk(expr.args[0]) && isNoClobberRisk(expr.args[1]) - } - - return false - } - - when { - call.args.all {isNoClobberRisk(it)} -> { - // There's no risk of clobbering for these simple argument types. Optimize the register loading directly from these values. - // register assignment order: 1) cx16 virtual word registers, 2) actual CPU registers, 3) CPU Carry status flag. - val argsInfo = sub.parameters.withIndex().zip(call.args).zip(sub.asmParameterRegisters) - val (cx16virtualRegs, args2) = argsInfo.partition { it.second.registerOrPair in Cx16VirtualRegisters } - val (cpuRegs, statusRegs) = args2.partition { it.second.registerOrPair!=null } - for(arg in cx16virtualRegs) - argumentViaRegister(sub, arg.first.first, arg.first.second) - for(arg in cpuRegs) - argumentViaRegister(sub, arg.first.first, arg.first.second) - for(arg in statusRegs) - argumentViaRegister(sub, arg.first.first, arg.first.second) - } - else -> { - // Risk of clobbering due to complex expression args. Evaluate first, then assign registers. - registerArgsViaStackEvaluation(call, sub) - } - } - } + asmgen.out(" jsr $subAsmName") } } - - val subName = asmgen.asmSymbolName(call.target) - if(!sub.inline || !asmgen.options.optimize) { - asmgen.out(" jsr $subName") - } else { - // inline the subroutine. - // we do this by copying the subroutine's statements at the call site. - // NOTE: *if* there is a return statement, it will be the only one, and the very last statement of the subroutine - // (this condition has been enforced by an ast check earlier) - - // note: for now, this is only reliably supported for asmsubs. - if(!sub.isAsmSubroutine) + else { + if(sub.inline) throw AssemblyError("can only reliably inline asmsub routines at this time") - asmgen.out(" \t; inlined routine follows: ${sub.name}") - val assembly = sub.statements.single() as InlineAssembly - asmgen.translate(assembly) - asmgen.out(" \t; inlined routine end: ${sub.name}") + argumentsViaVariables(sub, call) + asmgen.out(" jsr $subAsmName") } // remember: dealing with the X register and/or dealing with return values is the responsibility of the caller } + private fun argumentsViaVariables(sub: Subroutine, call: IFunctionCall) { + for(arg in sub.parameters.withIndex().zip(call.args)) + argumentViaVariable(sub, arg.first, arg.second) + } + + private fun argumentsViaRegisters(sub: Subroutine, call: IFunctionCall) { + fun isNoClobberRisk(expr: Expression): Boolean { + if(expr is AddressOf || + expr is NumericLiteralValue || + expr is StringLiteralValue || + expr is ArrayLiteralValue || + expr is IdentifierReference) + return true + + if(expr is FunctionCall) { + if(expr.target.nameInSource==listOf("lsb") || expr.target.nameInSource==listOf("msb")) + return isNoClobberRisk(expr.args[0]) + if(expr.target.nameInSource==listOf("mkword")) + return isNoClobberRisk(expr.args[0]) && isNoClobberRisk(expr.args[1]) + } + + return false + } + + if(sub.parameters.size==1) { + // just a single parameter, no risk of clobbering registers + argumentViaRegister(sub, IndexedValue(0, sub.parameters.single()), call.args[0]) + } else { + when { + call.args.all {isNoClobberRisk(it)} -> { + // There's no risk of clobbering for these simple argument types. Optimize the register loading directly from these values. + // register assignment order: 1) cx16 virtual word registers, 2) actual CPU registers, 3) CPU Carry status flag. + val argsInfo = sub.parameters.withIndex().zip(call.args).zip(sub.asmParameterRegisters) + val (cx16virtualRegs, args2) = argsInfo.partition { it.second.registerOrPair in Cx16VirtualRegisters } + val (cpuRegs, statusRegs) = args2.partition { it.second.registerOrPair!=null } + for(arg in cx16virtualRegs) + argumentViaRegister(sub, arg.first.first, arg.first.second) + for(arg in cpuRegs) + argumentViaRegister(sub, arg.first.first, arg.first.second) + for(arg in statusRegs) + argumentViaRegister(sub, arg.first.first, arg.first.second) + } + else -> { + // Risk of clobbering due to complex expression args. Evaluate first, then assign registers. + registerArgsViaStackEvaluation(call, sub) + } + } + } + } + private fun registerArgsViaStackEvaluation(stmt: IFunctionCall, sub: Subroutine) { // this is called when one or more of the arguments are 'complex' and // cannot be assigned to a register easily or risk clobbering other registers. diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AsmAssignment.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AsmAssignment.kt index 96f678e73..9fa86f229 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AsmAssignment.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AsmAssignment.kt @@ -65,7 +65,7 @@ internal class AsmAssignTarget(val kind: TargetStorageKind, identifier != null -> { val parameter = identifier!!.targetVarDecl(program)?.subroutineParameter if(parameter!=null && parameter.definingSubroutine!!.isAsmSubroutine) { - TODO("ASSIGN ASMPARAM $parameter :: $assign") + TODO("ASSIGNTARGET ASMPARAM $parameter :: $assign") } AsmAssignTarget(TargetStorageKind.VARIABLE, program, asmgen, dt, assign.definingSubroutine, variableAsmName = asmgen.asmVariableName(identifier!!), origAstTarget = this) } @@ -141,7 +141,7 @@ internal class AsmAssignSource(val kind: SourceStorageKind, is IdentifierReference -> { val parameter = value.targetVarDecl(program)?.subroutineParameter if(parameter!=null && parameter.definingSubroutine!!.isAsmSubroutine) { - TODO("ASSIGN SOURCE FROM ASMPARAM $parameter :: $value") + TODO("ASSIGNSOURCE ASMPARAM $parameter :: $value") } val dt = value.inferType(program).getOr(DataType.UNDEFINED) val varName=asmgen.asmVariableName(value) diff --git a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AugmentableAssignmentAsmGen.kt b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AugmentableAssignmentAsmGen.kt index 4b4c003c6..b20da9515 100644 --- a/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AugmentableAssignmentAsmGen.kt +++ b/codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AugmentableAssignmentAsmGen.kt @@ -236,9 +236,9 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, } } else -> { - // TODO OTHER EVALUATION HERE, don't use the estack + // TODO OTHER EVALUATION HERE, don't use the estack to transfer the address to read/write from asmgen.assignExpressionTo(memory.addressExpression, AsmAssignTarget(TargetStorageKind.STACK, program, asmgen, DataType.UWORD, memory.definingSubroutine)) - asmgen.out(" jsr prog8_lib.read_byte_from_address_on_stack | sta P8ZP_SCRATCH_B1") // TODO don't use estack to transfer the address to read from + asmgen.out(" jsr prog8_lib.read_byte_from_address_on_stack | sta P8ZP_SCRATCH_B1") when { valueLv != null -> inplaceModification_byte_litval_to_variable("P8ZP_SCRATCH_B1", DataType.UBYTE, operator, valueLv.toInt()) ident != null -> inplaceModification_byte_variable_to_variable("P8ZP_SCRATCH_B1", DataType.UBYTE, operator, ident) @@ -249,7 +249,7 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, } else -> inplaceModification_byte_value_to_variable("P8ZP_SCRATCH_B1", DataType.UBYTE, operator, value) } - asmgen.out(" lda P8ZP_SCRATCH_B1 | jsr prog8_lib.write_byte_to_address_on_stack | inx") // TODO don't use estack to transfer the address to read from + asmgen.out(" lda P8ZP_SCRATCH_B1 | jsr prog8_lib.write_byte_to_address_on_stack | inx") } } } @@ -1898,11 +1898,10 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, + ldx #1 +""") } - in Cx16VirtualRegisters -> TODO() + in Cx16VirtualRegisters -> throw AssemblyError("cx16 virtual regs should be variables, not real registers") else -> throw AssemblyError("invalid reg dt for word not") } } - TargetStorageKind.MEMORY -> TODO("no asm gen for uword-memory not") TargetStorageKind.STACK -> TODO("no asm gen for word stack not") else -> throw AssemblyError("no asm gen for in-place not of uword for ${target.kind}") } @@ -1974,11 +1973,10 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, RegisterOrPair.AX -> asmgen.out(" pha | txa | eor #255 | tax | pla | eor #255") RegisterOrPair.AY -> asmgen.out(" pha | tya | eor #255 | tay | pla | eor #255") RegisterOrPair.XY -> asmgen.out(" txa | eor #255 | tax | tya | eor #255 | tay") - in Cx16VirtualRegisters -> TODO("no asm gen for cx16 word register invert") + in Cx16VirtualRegisters -> throw AssemblyError("cx16 virtual regs should be variables, not real registers") else -> throw AssemblyError("invalid reg dt for word invert") } } - TargetStorageKind.MEMORY -> TODO("no asm gen for uword-memory invert") TargetStorageKind.STACK -> TODO("no asm gen for word stack invert") else -> throw AssemblyError("no asm gen for in-place invert uword for ${target.kind}") } @@ -2006,9 +2004,9 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, else -> throw AssemblyError("invalid reg dt for byte negate") } } - TargetStorageKind.MEMORY -> TODO("can't in-place negate memory ubyte") + TargetStorageKind.MEMORY -> throw AssemblyError("memory is ubyte, can't in-place negate") TargetStorageKind.STACK -> TODO("no asm gen for byte stack negate") - else -> throw AssemblyError("no asm gen for in-place negate byte array") + else -> throw AssemblyError("no asm gen for in-place negate byte") } } DataType.WORD -> { @@ -2063,13 +2061,12 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, sbc P8ZP_SCRATCH_REG+1 tay""") } - in Cx16VirtualRegisters -> TODO("no asm gen for cx16 word register negate") + in Cx16VirtualRegisters -> throw AssemblyError("cx16 virtual regs should be variables, not real registers") else -> throw AssemblyError("invalid reg dt for word neg") } } - TargetStorageKind.MEMORY -> TODO("no asm gen for word memory negate") TargetStorageKind.STACK -> TODO("no asm gen for word stack negate") - else -> throw AssemblyError("no asm gen for in-place negate word array") + else -> throw AssemblyError("no asm gen for in-place negate word") } } DataType.FLOAT -> { @@ -2082,8 +2079,6 @@ internal class AugmentableAssignmentAsmGen(private val program: Program, sta ${target.asmVarname}+1 """) } - TargetStorageKind.REGISTER -> TODO("no asm gen for float reg negate") - TargetStorageKind.MEMORY -> TODO("no asm gen for float memory negate") TargetStorageKind.STACK -> TODO("no asm gen for stack float negate") else -> throw AssemblyError("weird target kind for inplace negate float ${target.kind}") } diff --git a/compiler/res/prog8lib/cx16/syslib.p8 b/compiler/res/prog8lib/cx16/syslib.p8 index f6f15a647..8b697ede4 100644 --- a/compiler/res/prog8lib/cx16/syslib.p8 +++ b/compiler/res/prog8lib/cx16/syslib.p8 @@ -312,7 +312,7 @@ romsub $ff56 = joystick_get(ubyte joynr @A) -> ubyte @A, ubyte @X, ubyte @Y romsub $ff4d = clock_set_date_time(uword yearmonth @R0, uword dayhours @R1, uword minsecs @R2, ubyte jiffies @R3) clobbers(A, X, Y) romsub $ff50 = clock_get_date_time() clobbers(A, X, Y) -> uword @R0, uword @R1, uword @R2, ubyte @R3 ; result registers see clock_set_date_time() -; TODO specify the correct clobbers for alle these functions below, we now assume all 3 regs are clobbered +; TODO specify the correct clobbers for all functions below, we now assume all 3 regs are clobbered ; high level graphics & fonts romsub $ff20 = GRAPH_init(uword vectors @R0) clobbers(A,X,Y) diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 242191beb..6a4d59dd8 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -6,8 +6,9 @@ For next compiler release (7.4) Use GoSub to call subroutines (statements): - [DONE] allow separate assigns to subroutine's parameter variables / registers - [DONE] turn a regular subroutine call into assignments to the parameters + GoSub (take code from gosub branch) + - also do this for asmsubs taking >0 parameters -Function call expression: +Optimize Function calls in expressions: - move args to assignments to params - add tempvar immediately in front of expression with the fuction call - replace the function call in the expression with the tempvar diff --git a/examples/test.p8 b/examples/test.p8 index 921804e47..2ca2a21b5 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,23 +1,22 @@ -%import textio -%import conv main { sub start() { - ubyte @shared xx=20 - ubyte @shared yy=10 + &ubyte derp = $c000 - routine2() - routine2() - routine2() + @($c000) = not @($c000) + @($c000) = ~ @($c000) + ubyte uu + uu = abs(uu) + routine2(12345, true) repeat { - xx++ } + } - asmsub routine2() -> ubyte @A { + asmsub routine2(uword num @AY, ubyte switch @Pc) { %asm {{ adc #20 rts