From 98acff802f553e3c14f590f0122595ad46af92f2 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Thu, 4 Apr 2024 23:47:33 +0200 Subject: [PATCH] better checking for number of return values assignment optimization if return register already is the same as the assignment target --- codeCore/src/prog8/code/ast/AstExpressions.kt | 5 -- codeCore/src/prog8/code/ast/AstPrinter.kt | 2 +- codeCore/src/prog8/code/optimize/Optimizer.kt | 82 ++++++++++++++++++- .../cpu6502/assignment/AssignmentAsmGen.kt | 6 +- compiler/res/prog8lib/cx16/bmx.p8 | 4 +- compiler/res/prog8lib/cx16/diskio.p8 | 4 +- compiler/src/prog8/compiler/Compiler.kt | 14 ++-- .../compiler/astprocessing/AstChecker.kt | 10 +-- .../astprocessing/IntermediateAstMaker.kt | 4 +- docs/source/todo.rst | 6 +- examples/c64/tehtriz.p8 | 5 +- examples/cx16/chunkedfile/mcf.p8 | 2 +- examples/cx16/tehtriz.p8 | 5 +- examples/cx16/vtui/testvtui.p8 | 3 +- examples/test.p8 | 30 ++++--- 15 files changed, 134 insertions(+), 48 deletions(-) diff --git a/codeCore/src/prog8/code/ast/AstExpressions.kt b/codeCore/src/prog8/code/ast/AstExpressions.kt index b811cf22c..9bebd8abb 100644 --- a/codeCore/src/prog8/code/ast/AstExpressions.kt +++ b/codeCore/src/prog8/code/ast/AstExpressions.kt @@ -210,11 +210,6 @@ class PtFunctionCall(val name: String, val void: Boolean, type: DataType, position: Position) : PtExpression(type, position) { - init { - if(!void) - require(type!=DataType.UNDEFINED) - } - val args: List get() = children.map { it as PtExpression } } diff --git a/codeCore/src/prog8/code/ast/AstPrinter.kt b/codeCore/src/prog8/code/ast/AstPrinter.kt index c9294928c..86c1d7b66 100644 --- a/codeCore/src/prog8/code/ast/AstPrinter.kt +++ b/codeCore/src/prog8/code/ast/AstPrinter.kt @@ -10,7 +10,7 @@ fun printAst(root: PtNode, skipLibraries: Boolean, output: (text: String) -> Uni fun type(dt: DataType) = "!${dt.name.lowercase()}!" fun txt(node: PtNode): String { return when(node) { - is PtAssignTarget -> "" + is PtAssignTarget -> if(node.void) "" else "" is PtAssignment -> "" is PtAugmentedAssign -> " ${node.operator}" is PtBreakpoint -> "%breakpoint" diff --git a/codeCore/src/prog8/code/optimize/Optimizer.kt b/codeCore/src/prog8/code/optimize/Optimizer.kt index 5113844bc..8a2a3a6f3 100644 --- a/codeCore/src/prog8/code/optimize/Optimizer.kt +++ b/codeCore/src/prog8/code/optimize/Optimizer.kt @@ -1,13 +1,18 @@ package prog8.code.optimize +import prog8.code.StRomSub +import prog8.code.SymbolTable import prog8.code.ast.* import prog8.code.core.* -fun optimizeIntermediateAst(program: PtProgram, options: CompilationOptions, errors: IErrorReporter) { +fun optimizeIntermediateAst(program: PtProgram, options: CompilationOptions, st: SymbolTable, errors: IErrorReporter) { if (!options.optimize) return - while(errors.noErrors() && optimizeCommonSubExpressions(program, errors)>0) { + while (errors.noErrors() && + (optimizeCommonSubExpressions(program, errors) + + optimizeAssignTargets(program, st, errors)) > 0 + ) { // keep rolling } } @@ -109,6 +114,79 @@ private fun optimizeCommonSubExpressions(program: PtProgram, errors: IErrorRepor } +private fun optimizeAssignTargets(program: PtProgram, st: SymbolTable, errors: IErrorReporter): Int { + var changes = 0 + walkAst(program) { node: PtNode, depth: Int -> + if(node is PtAssignment) { + val value = node.value + val functionName = when(value) { + is PtBuiltinFunctionCall -> value.name + is PtFunctionCall -> value.name + else -> null + } + if(functionName!=null) { + val stNode = st.lookup(functionName) + if (stNode is StRomSub) { + require(node.children.size==stNode.returns.size+1) { + "number of targets must match return values" + } + node.children.zip(stNode.returns).withIndex().forEach { (index, xx) -> + val target = xx.first as PtAssignTarget + val returnedRegister = xx.second.register.registerOrPair + if(returnedRegister!=null && !target.void && target.identifier!=null) { + if(isSame(target.identifier!!, xx.second.type, returnedRegister)) { + // output register is already identical to target register, so it can become void + val voidTarget = PtAssignTarget(true, target.position) + node.children[index] = voidTarget + voidTarget.parent = node + changes++ + } + } + } + } + if(node.children.dropLast(1).all { (it as PtAssignTarget).void }) { + // all targets are now void, the whole assignment can be discarded and replaced by just a (void) call to the subroutine + val index = node.parent.children.indexOf(node) + val voidCall = PtFunctionCall(functionName, true, value.type, value.position) + value.children.forEach { voidCall.add(it) } + node.parent.children[index] = voidCall + voidCall.parent = node.parent + changes++ + } + } + } + true + } + return changes +} + +internal fun isSame(identifier: PtIdentifier, type: DataType, returnedRegister: RegisterOrPair): Boolean { + if(returnedRegister in Cx16VirtualRegisters) { + val regname = returnedRegister.name.lowercase() + val identifierRegName = identifier.name.substringAfterLast('.') + /* + cx16.r? UWORD + cx16.r?s WORD + cx16.r?L UBYTE + cx16.r?H UBYTE + cx16.r?sL BYTE + cx16.r?sH BYTE + */ + if(identifier.type in ByteDatatypes && type in ByteDatatypes) { + if(identifier.name.startsWith("cx16.$regname") && identifierRegName.startsWith(regname)) { + return identifierRegName.substring(2) in arrayOf("", "L", "sL") // note: not the -H (msb) variants! + } + } + else if(identifier.type in WordDatatypes && type in WordDatatypes) { + if(identifier.name.startsWith("cx16.$regname") && identifierRegName.startsWith(regname)) { + return identifierRegName.substring(2) in arrayOf("", "s") + } + } + } + return false // there are no identifiers directly corresponding to cpu registers +} + + internal fun findScopeName(node: PtNode): String { var parent=node while(parent !is PtNamedNode) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt index 179c9154b..34fad857f 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt @@ -112,7 +112,11 @@ internal class AssignmentAsmGen(private val program: PtProgram, val tgt = AsmAssignTarget.fromAstAssignment(target, target.definingISub(), asmgen) when(returns.type) { in ByteDatatypesWithBoolean -> { - assignRegisterByte(tgt, returns.register.registerOrPair!!.asCpuRegister(), false, false) + if(returns.register.registerOrPair in Cx16VirtualRegisters) { + assignVirtualRegister(tgt, returns.register.registerOrPair!!) + } else { + assignRegisterByte(tgt, returns.register.registerOrPair!!.asCpuRegister(), false, false) + } } in WordDatatypes -> { assignRegisterpairWord(tgt, returns.register.registerOrPair!!) diff --git a/compiler/res/prog8lib/cx16/bmx.p8 b/compiler/res/prog8lib/cx16/bmx.p8 index d076cb64f..bdac5c13b 100644 --- a/compiler/res/prog8lib/cx16/bmx.p8 +++ b/compiler/res/prog8lib/cx16/bmx.p8 @@ -235,7 +235,7 @@ save_end: sub read_scanline(uword size) { while size!=0 { - cx16.r0 = cx16.MACPTR(min(255, size) as ubyte, &cx16.VERA_DATA0, true) + void, cx16.r0 = cx16.MACPTR(min(255, size) as ubyte, &cx16.VERA_DATA0, true) if_cs { ; no MACPTR support repeat size @@ -297,7 +297,7 @@ save_end: cx16.r0L = lsb(size) if msb(size)!=0 cx16.r0L = 0 ; 256 bytes - cx16.r0 = cx16.MCIOUT(cx16.r0L, &cx16.VERA_DATA0, true) + void, cx16.r0 = cx16.MCIOUT(cx16.r0L, &cx16.VERA_DATA0, true) if_cs { ; no MCIOUT support repeat size diff --git a/compiler/res/prog8lib/cx16/diskio.p8 b/compiler/res/prog8lib/cx16/diskio.p8 index 5568c570a..94eea9a16 100644 --- a/compiler/res/prog8lib/cx16/diskio.p8 +++ b/compiler/res/prog8lib/cx16/diskio.p8 @@ -331,7 +331,7 @@ close_end: readsize = 255 if num_bytes rr.registerOrPair != null } - if(returnRegisters.size>1) { - errors.err("multiple return values and too few assignment targets, need at least ${returnRegisters.size}", fcall.position) - } + errors.err("number of assignment targets doesn't match number of return values from the subroutine", fcall.position) + return } } @@ -600,7 +596,7 @@ internal class AstChecker(private val program: Program, } val targets = assignment.target.multi!! if(fcallTarget.returntypes.size!=targets.size) { - errors.err("number of assignment targets doesn't match number of return values", fcall.position) + errors.err("number of assignment targets doesn't match number of return values from the subroutine", fcall.position) return } fcallTarget.returntypes.zip(targets).withIndex().forEach { (index, p) -> diff --git a/compiler/src/prog8/compiler/astprocessing/IntermediateAstMaker.kt b/compiler/src/prog8/compiler/astprocessing/IntermediateAstMaker.kt index ebc2e1c13..4cfbb0689 100644 --- a/compiler/src/prog8/compiler/astprocessing/IntermediateAstMaker.kt +++ b/compiler/src/prog8/compiler/astprocessing/IntermediateAstMaker.kt @@ -270,7 +270,7 @@ class IntermediateAstMaker(private val program: Program, private val errors: IEr private fun transform(srcCall: FunctionCallStatement): PtFunctionCall { val (target, type) = srcCall.target.targetNameAndType(program) - val call = PtFunctionCall(target,true, type, srcCall.position) + val call = PtFunctionCall(target, true, type, srcCall.position) for (arg in srcCall.args) call.add(transformExpression(arg)) return call @@ -279,7 +279,7 @@ class IntermediateAstMaker(private val program: Program, private val errors: IEr private fun transform(srcCall: FunctionCallExpression): PtFunctionCall { val (target, _) = srcCall.target.targetNameAndType(program) val iType = srcCall.inferType(program) - val call = PtFunctionCall(target, iType.isUnknown, iType.getOrElse { DataType.UNDEFINED }, srcCall.position) + val call = PtFunctionCall(target, iType.isUnknown && srcCall.parent !is Assignment, iType.getOrElse { DataType.UNDEFINED }, srcCall.position) for (arg in srcCall.args) call.add(transformExpression(arg)) return call diff --git a/docs/source/todo.rst b/docs/source/todo.rst index e546d5831..5d37ebe87 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,9 +1,9 @@ TODO ==== -remove redundant assignments in calls like this where the result registers are the same as the assigment targets: -void, cx16.r0s, cx16.r1s = cx16.mouse_pos() -(6502 + IR) +return cx16.MACPTR(0, 2, true) -> should give compiler error about number of values + +check docs on assign about status register in assignment (can no longer be ignored, use void to not assign it) ... diff --git a/examples/c64/tehtriz.p8 b/examples/c64/tehtriz.p8 index 38b7b0802..2ad81a99f 100644 --- a/examples/c64/tehtriz.p8 +++ b/examples/c64/tehtriz.p8 @@ -73,7 +73,8 @@ waitkey: ; test_stack.test() } - ubyte key=cbm.GETIN() + ubyte key + void, key=cbm.GETIN() if key==0 goto waitkey keypress(key) @@ -252,7 +253,7 @@ waitkey: ubyte key = 0 while key!=133 { ; endless loop until user presses F1 to restart the game - key = cbm.GETIN() + void, key = cbm.GETIN() } } diff --git a/examples/cx16/chunkedfile/mcf.p8 b/examples/cx16/chunkedfile/mcf.p8 index 02bfe3a31..e264e4390 100644 --- a/examples/cx16/chunkedfile/mcf.p8 +++ b/examples/cx16/chunkedfile/mcf.p8 @@ -182,7 +182,7 @@ processchunk_call jsr $ffff ; modified ubyte readsize = 255 if msb(size)==0 readsize = lsb(size) - cx16.r2 = cx16.MACPTR(readsize, bonkbuffer, false) ; can't MACPTR directly to bonk ram + void, cx16.r2 = cx16.MACPTR(readsize, bonkbuffer, false) ; can't MACPTR directly to bonk ram cx16.rombank(bonk) sys.memcopy(bonkbuffer, cx16.r3, cx16.r2) ; copy to bonk ram cx16.rombank(orig_rom_bank) diff --git a/examples/cx16/tehtriz.p8 b/examples/cx16/tehtriz.p8 index caa90f591..1064def96 100644 --- a/examples/cx16/tehtriz.p8 +++ b/examples/cx16/tehtriz.p8 @@ -95,7 +95,8 @@ waitkey: ; test_stack.test() } - ubyte key=cbm.GETIN() + ubyte key + void, key=cbm.GETIN() keypress(key) cx16.r0,void = cx16.joystick_get(1) joystick(cx16.r0) @@ -338,7 +339,7 @@ waitkey: cx16.r0, void = cx16.joystick_get(1) if cx16.r0 & %0000000000010000 == 0 break - key = cbm.GETIN() + void, key = cbm.GETIN() } until key==133 } diff --git a/examples/cx16/vtui/testvtui.p8 b/examples/cx16/vtui/testvtui.p8 index 8e5bf454c..8597f6405 100644 --- a/examples/cx16/vtui/testvtui.p8 +++ b/examples/cx16/vtui/testvtui.p8 @@ -66,7 +66,8 @@ main { vtui.print_str2("arrow keys to move!", $61, true) char_loop: - ubyte char = cbm.GETIN() + ubyte char + void, char = cbm.GETIN() if char==0 goto char_loop diff --git a/examples/test.p8 b/examples/test.p8 index 1f0785d0a..57f33969e 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,19 +1,25 @@ -%import textio %zeropage basicsafe %option no_sysinit main { - sub start() { - cx16.mouse_config2(1) - wait_mousebutton() + romsub $2000 = func1() clobbers(X) -> ubyte @A, word @R0, byte @R1 + romsub $3000 = func2() clobbers(X) -> ubyte @A, uword @R0, uword @R1 + romsub $4000 = func3() clobbers(X) -> ubyte @R0 - sub wait_mousebutton() { - do { - cx16.r0L, void, void = cx16.mouse_pos() - } until cx16.r0L!=0 - do { - cx16.r0L, void, void = cx16.mouse_pos() - } until cx16.r0L==0 - } + sub start() { + bool flag + void cbm.GETIN() + flag, cx16.r1L = cbm.GETIN() + + void, cx16.r0s, cx16.r1sL = func1() + void, cx16.r2, cx16.r1 = func2() + cx16.r0L = func3() + cx16.r0H = func3() + + cx16.r0 = readblock() + } + + sub readblock() -> uword { + return cx16.MACPTR(0, 2, true) ; TODO compiler error (number of return values) } }