From f70b914779947aad3a54513bfcb0ecd1b0c0bba1 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Mon, 6 Jun 2022 13:21:45 +0200 Subject: [PATCH] fix optimized codegen for 2 arg functions, sometimes was passing wrong arg value due to register overwriting --- .../codegen/cpu6502/BuiltinFunctionsAsmGen.kt | 7 ++- .../codegen/cpu6502/FunctionCallAsmGen.kt | 13 +++--- .../astprocessing/AstOnetimeTransforms.kt | 8 +--- docs/source/todo.rst | 2 +- examples/test.p8 | 46 ++++++++++++------- 5 files changed, 44 insertions(+), 32 deletions(-) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/BuiltinFunctionsAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/BuiltinFunctionsAsmGen.kt index fa0e2d5e2..c202a17b6 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/BuiltinFunctionsAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/BuiltinFunctionsAsmGen.kt @@ -1384,12 +1384,12 @@ internal class BuiltinFunctionsAsmGen(private val program: Program, asmgen.out(" sta P8ESTACK_LO,x | tya | sta P8ESTACK_HI,x | dex") } else { val reg = resultRegister ?: RegisterOrPair.AY - var needAsave = !(fcall.args[0] is DirectMemoryRead || fcall.args[0] is NumericLiteral || fcall.args[0] is IdentifierReference) + var needAsave = needAsaveForOtherArg(fcall.args[0]) if(!needAsave) { val mr0 = fcall.args[0] as? DirectMemoryRead val mr1 = fcall.args[1] as? DirectMemoryRead if (mr0 != null) - needAsave = mr0.addressExpression !is NumericLiteral && mr0.addressExpression !is IdentifierReference + needAsave = mr0.addressExpression !is NumericLiteral && mr0.addressExpression !is IdentifierReference if (mr1 != null) needAsave = needAsave or (mr1.addressExpression !is NumericLiteral && mr1.addressExpression !is IdentifierReference) } @@ -1633,4 +1633,7 @@ internal class BuiltinFunctionsAsmGen(private val program: Program, } } + private fun needAsaveForOtherArg(arg: Expression): Boolean = + arg !is NumericLiteral && arg !is IdentifierReference && arg !is DirectMemoryRead + } diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt index d4cc8f9f9..b5288da17 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt @@ -3,10 +3,7 @@ package prog8.codegen.cpu6502 import prog8.ast.IFunctionCall import prog8.ast.Node import prog8.ast.Program -import prog8.ast.expressions.AddressOf -import prog8.ast.expressions.Expression -import prog8.ast.expressions.IdentifierReference -import prog8.ast.expressions.NumericLiteral +import prog8.ast.expressions.* import prog8.ast.statements.* import prog8.code.core.* import prog8.codegen.cpu6502.assignment.AsmAssignSource @@ -72,6 +69,10 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg (sub.parameters.size==1 && sub.parameters[0].type in IntegerDatatypes) || (sub.parameters.size==2 && sub.parameters[0].type in ByteDatatypes && sub.parameters[1].type in ByteDatatypes) + private fun needAsaveForOtherArg(arg: Expression): Boolean = + arg !is NumericLiteral && arg !is IdentifierReference && arg !is DirectMemoryRead + + internal fun translateFunctionCall(call: IFunctionCall, isExpression: Boolean) { // Output only the code to set up the parameters and perform the actual call // NOTE: does NOT output the code to deal with the result values! @@ -111,10 +112,10 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg } else { // 2 byte params, second in Y, first in A argumentViaRegister(sub, IndexedValue(0, sub.parameters[0]), call.args[0], RegisterOrPair.A) - if(!call.args[1].isSimple) + if(needAsaveForOtherArg(call.args[1])) asmgen.out(" pha") argumentViaRegister(sub, IndexedValue(1, sub.parameters[1]), call.args[1], RegisterOrPair.Y) - if(!call.args[1].isSimple) + if(needAsaveForOtherArg(call.args[1])) asmgen.out(" pla") } } else { diff --git a/compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt b/compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt index 15ca750e8..26a3c82e7 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstOnetimeTransforms.kt @@ -50,12 +50,8 @@ internal class AstOnetimeTransforms(private val program: Program, private val op } else { val fcall = parent as? IFunctionCall if(fcall!=null) { - if(options.compTarget.name != VMTarget.NAME) { - // TODO for now, 6502 codegen seems wrong when using pointer indexed args to any function. - val memread = DirectMemoryRead(add, arrayIndexedExpression.position) - return listOf(IAstModification.ReplaceNode(arrayIndexedExpression, memread, parent)) - } else if(fcall.target.nameInSource.size==1 && fcall.target.nameInSource[0] in InplaceModifyingBuiltinFunctions) { - // TODO for now, vm codegen seems wrong when using pointer indexed args to an in-place modifying function. + // TODO for now, codegen seems wrong when using pointer indexed args to an in-place modifying function. + if(fcall.target.nameInSource.size==1 && fcall.target.nameInSource[0] in InplaceModifyingBuiltinFunctions) { val memread = DirectMemoryRead(add, arrayIndexedExpression.position) return listOf(IAstModification.ReplaceNode(arrayIndexedExpression, memread, parent)) } else { diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 1709291eb..01e9b1bec 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,7 +3,7 @@ TODO For next release ^^^^^^^^^^^^^^^^ -- optimize pointervar indexing codegen: make them work as subroutine paramers +- optimize pointervar indexing codegen: make them work as params to in-place modifying functions (rol, swap, ...) - optimize pointervar indexing codegen: writing (all sorts of things) - pipe operator: (targets other than 'Virtual'): allow non-unary function calls in the pipe that specify the other argument(s) in the calls. Already working for VM target. - add McCarthy evaluation to shortcircuit and/or expressions. First do ifs by splitting them up? Then do expressions that compute a value? diff --git a/examples/test.p8 b/examples/test.p8 index 40719b66f..47633b07a 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -29,27 +29,39 @@ main { ; txt.spc() ; } +; sub derp(ubyte u1, ubyte u2, ubyte u3) { +; txt.print_ub(u1) +; txt.spc() +; txt.print_ub(u2) +; txt.spc() +; txt.print_ub(u3) +; txt.nl() +; } + + sub derp2(ubyte u1, ubyte u2) { + txt.print_ub(u1) + txt.spc() + txt.print_ub(u2) + txt.nl() + } + sub start() { ; mcCarthy() - uword uw1 = 9999 - uword uw2 = uw1 - 2000 + 10000 - ubyte ub1 = 99 - ubyte ub2 = ub1 - 2 + 10 - ubyte ubb = ub2 - -10 - txt.print_uw(uw2) - txt.spc() - txt.print_ub(ub2) - txt.spc() - txt.print_ub(ubb) - txt.nl() - ;test_stack.test() -; ubyte value = 0 -; ubyte one = 1 -; ubyte[10] data = [11,22,33,4,5,6,7,8,9,10] -; uword bitmapbuf = &data -; + ubyte value = 0 + ubyte one = 1 + ubyte two = 2 + ubyte[10] data = [11,22,33,4,5,6,7,8,9,10] + uword bitmapbuf = &data + + ;derp(data[0],data[1],data[2]) + ;derp(bitmapbuf[0], bitmapbuf[1], bitmapbuf[2]) + derp2(1,2) + derp2(one, two) + derp2(data[1], data[2]) + derp2(bitmapbuf[1], bitmapbuf[2]) + ; value = bitmapbuf[2] ; txt.print_ub(value) ;; 33 ; txt.nl()