From 526e4b8bdcfd5a4a6bad84c7b5f5b914cf297bc3 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Tue, 3 Nov 2020 21:31:08 +0100 Subject: [PATCH] fix faulty binexpr splitting --- .../c64/codegen/BuiltinFunctionsAsmGen.kt | 23 ++++++++------- .../src/prog8/optimizer/BinExprSplitter.kt | 25 +++++++++++------ examples/plasma.p8 | 3 +- examples/test.p8 | 28 +++++++++---------- 4 files changed, 45 insertions(+), 34 deletions(-) diff --git a/compiler/src/prog8/compiler/target/c64/codegen/BuiltinFunctionsAsmGen.kt b/compiler/src/prog8/compiler/target/c64/codegen/BuiltinFunctionsAsmGen.kt index dd3355459..7f4d992db 100644 --- a/compiler/src/prog8/compiler/target/c64/codegen/BuiltinFunctionsAsmGen.kt +++ b/compiler/src/prog8/compiler/target/c64/codegen/BuiltinFunctionsAsmGen.kt @@ -1053,25 +1053,28 @@ internal class BuiltinFunctionsAsmGen(private val program: Program, private val when { conv.variable -> { val varname = "prog8_lib.func_${signature.name}_cc._arg_${paramName}" // TODO after all builtin funcs have been changed into _cc, remove that suffix again - val src = AsmAssignSource.fromAstSource(value, program, asmgen) + val src = if(conv.dt==DataType.FLOAT || conv.dt in PassByReferenceDatatypes) { + // put the address of the argument in AY + val addr = AddressOf(value as IdentifierReference, value.position) + AsmAssignSource.fromAstSource(addr, program, asmgen) + } else { + AsmAssignSource.fromAstSource(value, program, asmgen) + } val tgt = AsmAssignTarget(TargetStorageKind.VARIABLE, program, asmgen, conv.dt, null, variableAsmName = varname) val assign = AsmAssignment(src, tgt, false, value.position) asmgen.translateNormalAssignment(assign) } conv.reg != null -> { - if(conv.dt==DataType.FLOAT || conv.dt in PassByReferenceDatatypes) { + val src = if(conv.dt==DataType.FLOAT || conv.dt in PassByReferenceDatatypes) { // put the address of the argument in AY val addr = AddressOf(value as IdentifierReference, value.position) - val src = AsmAssignSource.fromAstSource(addr, program, asmgen) - val tgt = AsmAssignTarget.fromRegisters(conv.reg, null, program, asmgen) - val assign = AsmAssignment(src, tgt, false, value.position) - asmgen.translateNormalAssignment(assign) + AsmAssignSource.fromAstSource(addr, program, asmgen) } else { - val src = AsmAssignSource.fromAstSource(value, program, asmgen) - val tgt = AsmAssignTarget.fromRegisters(conv.reg, null, program, asmgen) - val assign = AsmAssignment(src, tgt, false, value.position) - asmgen.translateNormalAssignment(assign) + AsmAssignSource.fromAstSource(value, program, asmgen) } + val tgt = AsmAssignTarget.fromRegisters(conv.reg, null, program, asmgen) + val assign = AsmAssignment(src, tgt, false, value.position) + asmgen.translateNormalAssignment(assign) } else -> throw AssemblyError("callconv") } diff --git a/compiler/src/prog8/optimizer/BinExprSplitter.kt b/compiler/src/prog8/optimizer/BinExprSplitter.kt index c3eb06a2d..787419b52 100644 --- a/compiler/src/prog8/optimizer/BinExprSplitter.kt +++ b/compiler/src/prog8/optimizer/BinExprSplitter.kt @@ -38,22 +38,26 @@ internal class BinExprSplitter(private val program: Program) : AstWalker() { if (binExpr != null) { /* -reduce the complexity of a (binary) expression that has to be evaluated on the eval stack, -by attempting to splitting it up into individual simple steps: +Reduce the complexity of a (binary) expression that has to be evaluated on the eval stack, +by attempting to splitting it up into individual simple steps. +We only consider a binary expression *one* level deep (so the operands must not be a combined expression) -X = BinExpr X = LeftExpr - followed by - / \ IF 'X' not used X = BinExpr - / \ IN LEFTEXPR ==> +X = BinExpr X = LeftExpr + followed by + / \ IF 'X' not used X = BinExpr + / \ IN expression ==> / \ / \ LeftExpr. RightExpr. / \ - / \ / \ X RightExpr. - .. .. .. .. + X RightExpr. + */ if(binExpr.operator in augmentAssignmentOperators && isSimpleTarget(assignment.target, program.namespace)) { - if (!assignment.isAugmentable) { + if(assignment.target isSameAs binExpr.left || assignment.target isSameAs binExpr.right) + return noModifications + + if(isSimpleExpression(binExpr.left) && isSimpleExpression(binExpr.right) && !assignment.isAugmentable) { val firstAssign = Assignment(assignment.target, binExpr.left, binExpr.left.position) val targetExpr = assignment.target.toExpression() val augExpr = BinaryExpression(targetExpr, binExpr.operator, binExpr.right, binExpr.right.position) @@ -71,6 +75,9 @@ X = BinExpr X = LeftExpr return noModifications } + private fun isSimpleExpression(expr: Expression) = + expr is IdentifierReference || expr is NumericLiteralValue || expr is AddressOf || expr is DirectMemoryRead || expr is StringLiteralValue || expr is ArrayLiteralValue || expr is RangeExpr + private fun isSimpleTarget(target: AssignTarget, namespace: INameScope) = if (target.identifier!=null || target.memoryAddress!=null || target.arrayindexed!=null) target.isInRegularRAM(namespace) diff --git a/examples/plasma.p8 b/examples/plasma.p8 index b2bf445b8..e18205f79 100644 --- a/examples/plasma.p8 +++ b/examples/plasma.p8 @@ -8,9 +8,10 @@ ; Cleanup and porting to C by Ullrich von Bassewitz. ; Converted to prog8 by Irmen de Jong. - +; TODO the random charset is all wrong ; TODO why has the prg become bigger since register args? + main { const uword SCREEN1 = $E000 const uword SCREEN2 = $E400 diff --git a/examples/test.p8 b/examples/test.p8 index c7bf03fbc..ec1bed67d 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -12,28 +12,28 @@ main { const uword ADDR = $0400 byte zerob=0 - word zerow=0 - float zerof=0 +; word zerow=0 +; float zerof=0 byte bb - word ww - float fl +; word ww +; float fl testX() bb = -100 - bb = zerob+abs(bb) + bb = zerob+abs(bb) ; TODO optimizer generates wrong code for this (wrong order of splitted expression?) txt.print_b(bb) txt.chrout('\n') - ww = -12345 - ww = zerow+abs(ww) - txt.print_w(ww) - txt.chrout('\n') - - fl = -9.876 - fl = zerof+abs(fl) - floats.print_f(fl) - txt.chrout('\n') +; ww = -12345 +; ww = zerow+abs(ww) ; TODO optimizer generates wrong code for this (wrong order of splitted expression?) +; txt.print_w(ww) +; txt.chrout('\n') +; +; fl = -9.876 +; fl = zerof+abs(fl) ; TODO optimizer generates wrong code for this (wrong order of splitted expression?) +; floats.print_f(fl) +; txt.chrout('\n') ; memset(ADDR, 40*25, 100) ; memsetw(ADDR, 20*10, $3031)