From c3fbdf34ca0436258f2f9d297f3c3bad9be27add Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sun, 3 Sep 2023 16:40:10 +0200 Subject: [PATCH] fixed c64 float problem --- .../src/prog8/codegen/cpu6502/AsmGen.kt | 6 +- .../cpu6502/assignment/AnyExprAsmGen.kt | 29 +++++-- compiler/res/prog8lib/c64/floats.asm | 10 ++- docs/source/todo.rst | 5 -- floatproblem64-small.asm | 80 ------------------- floatproblem64.p8 | 57 ------------- 6 files changed, 36 insertions(+), 151 deletions(-) delete mode 100644 floatproblem64-small.asm delete mode 100644 floatproblem64.p8 diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt index 7f28cca5b..6144066cd 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt @@ -3031,7 +3031,11 @@ $repeatLabel""") } internal fun popFAC1() { - out(" jsr floats.popFAC1") + out(" clc | jsr floats.popFAC") + } + + internal fun popFAC2() { + out(" sec | jsr floats.popFAC") } internal fun needAsaveForExpr(arg: PtExpression): Boolean = diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AnyExprAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AnyExprAsmGen.kt index cd0fae209..406d894c2 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AnyExprAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AnyExprAsmGen.kt @@ -3,6 +3,8 @@ package prog8.codegen.cpu6502.assignment import prog8.code.ast.PtBinaryExpression import prog8.code.ast.PtExpression import prog8.code.core.* +import prog8.code.target.C64Target +import prog8.code.target.Cx16Target import prog8.codegen.cpu6502.AsmGen6502Internal // @@ -245,11 +247,28 @@ internal class AnyExprAsmGen( } private fun assignFloatOperandsToFACandARG(left: PtExpression, right: PtExpression) { - asmgen.assignExpressionToRegister(left, RegisterOrPair.FAC1, true) - if(!right.isSimple()) asmgen.pushFAC1() - asmgen.assignExpressionToRegister(right, RegisterOrPair.FAC2, true) - if(!right.isSimple()) asmgen.popFAC1() - // TODO: always make sure FAC2 is loaded last (done using CONUPK) otherwise the result will be corrupt on C64 + when(asmgen.options.compTarget.name) { + C64Target.NAME -> { + // c64 has a quirk: always make sure FAC2 is loaded last (done using CONUPK) otherwise the result will be corrupt on C64 + // this requires some more forced copying around of float values in certain cases + if (right.isSimple()) { + asmgen.assignExpressionToRegister(left, RegisterOrPair.FAC1, true) + asmgen.assignExpressionToRegister(right, RegisterOrPair.FAC2, true) + } else { + asmgen.assignExpressionToRegister(right, RegisterOrPair.FAC1, true) + asmgen.pushFAC1() + asmgen.assignExpressionToRegister(left, RegisterOrPair.FAC1, true) + asmgen.popFAC2() + } + } + Cx16Target.NAME -> { + asmgen.assignExpressionToRegister(left, RegisterOrPair.FAC1, true) + if (!right.isSimple()) asmgen.pushFAC1() + asmgen.assignExpressionToRegister(right, RegisterOrPair.FAC2, true) + if (!right.isSimple()) asmgen.popFAC1() + } + else -> TODO("don't know how to evaluate float expression for selected compilation target") + } } private fun setupFloatComparisonFAC1vsVarAY(expr: PtBinaryExpression) { diff --git a/compiler/res/prog8lib/c64/floats.asm b/compiler/res/prog8lib/c64/floats.asm index e118a0824..0040a0309 100644 --- a/compiler/res/prog8lib/c64/floats.asm +++ b/compiler/res/prog8lib/c64/floats.asm @@ -418,8 +418,9 @@ pushFAC1 .proc rts .pend -popFAC1 .proc - ; -- pop floating point value from cpu stack into FAC1 +popFAC .proc + ; -- pop floating point value from cpu stack into FAC1 or FAC2 ( + ; carry flag clear=FAC1, carry set=FAC2 ; save return address pla sta P8ZP_SCRATCH_W2 @@ -437,8 +438,11 @@ popFAC1 .proc sta floats.floats_temp_var lda #floats.floats_temp_var + bcs + jsr floats.MOVFM - ; re-push return address + jmp ++ ++ jsr floats.CONUPK ++ ; re-push return address lda P8ZP_SCRATCH_W2+1 pha lda P8ZP_SCRATCH_W2 diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 35114c226..b39bf7e97 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,10 +1,5 @@ TODO ==== -- fix on c64 target: examples/cube3d-float (broken since 9.3 with the evalstack removal) it works on x16 target, oddly enough. - More detailed and simpler code for this problem in floatproblem.p8 / floatproblem64-small.asm (the minified version) - Seems like MOVFM (loading FAC) corrupts the sign of ARG, so CONUPK (loading ARG) needs to be done AFTER MOVFM (loading FAC)...? - See assignFloatOperandsToFACandARG() - - prefix prog8 subroutines with p8s_ instead of p8_ to not let them clash with variables in the asm? - allow 'chained' array indexing for expressions: value = ptrarray[0][0] - allow 'chained' array indexing for assign targets: ptrarray[0][0] = 42 this is just evaluating the lhs as a uword pointer expression diff --git a/floatproblem64-small.asm b/floatproblem64-small.asm deleted file mode 100644 index 74c5ffb4a..000000000 --- a/floatproblem64-small.asm +++ /dev/null @@ -1,80 +0,0 @@ - -.cpu '6502' - -CHROUT = $FFD2 - -; C64 addresses for float routines: -MOVFM = $bba2 -CONUPK = $ba8c -MOVAF = $bc0c -MOVEF = $bc0f -MOVMF = $bbd4 -FOUT = $bddd -FADDT = $b86a -FMULTT = $ba2b -; CX16 addresses for float routines: -;MOVFM = $fe63 -;CONUPK = $fe5a -;MOVAF = $fe6c -;MOVEF = $fe81 -;MOVMF = $fe66 -;FOUT = $fe06 -;FADDT = $fe1b -;FMULTT = $fe21 - -* = $0801 - -; 10 SYS 2062 - .word (+), 10 - .null $9e, " 2062" -+ .word 0 - -; program starts here at address 2062 - lda #$8d - jsr CHROUT - - ldx #0 - stx $6f - lda #prog8_float_const_1 - jsr CONUPK ; ARG = 0.3 - lda #prog8_float_const_0 - jsr MOVFM ; FAC1 = -0.8 ; On C64 this also seems to corrupt the value in ARG!!! (or at least, the sign of that) - jsr FADDT ; FAC1 = -0.8 + 0.3 = -0.5 - ; ... result in FAC1 will be wrong on C64 (1.1 instead of -0.5).... - ; Seems like MOVFM (loading FAC) corrupts the sign of ARG, so CONUPK (loading ARG) needs to be done AFTER MOVFM (loading FAC)...? - - ; print FAC1 - jsr FOUT ; fac1 to string in A/Y - sta _mod+1 - sty _mod+2 - ldy #0 -_mod lda $ffff,y ; modified - beq + - jsr CHROUT - iny - bne _mod -+ lda #$8d - jsr CHROUT - - ; print expected - ldy #0 -- lda string_1,y - beq + - jsr CHROUT - iny - bne - -+ - -loop jmp loop - - -string_1 - .text "-.5 was expected",$8d,0 - - -; global float constants -prog8_float_const_0 .byte $80, $cc, $cc, $cc, $cc ; float -0.8 -prog8_float_const_1 .byte $7f, $19, $99, $99, $99 ; float 0.3 -float_const_one .byte $81, $00, $00, $00, $00 ; float 1.0 diff --git a/floatproblem64.p8 b/floatproblem64.p8 deleted file mode 100644 index 23dc94c6a..000000000 --- a/floatproblem64.p8 +++ /dev/null @@ -1,57 +0,0 @@ -%import textio -%import floats -%import test_stack -%zeropage dontuse - -main { - sub start() { - float value1 = -0.8 - float value2 = 0.3 - float one = 1.0 - - float result = value1*one + value2*one ; TODO FIX: invalid result on c64, ok when the *one is removed or expression is split (it's not caused by pushFAC1/popFAC1) - ; TODO is it floats.CONUPK?? - floats.print_f(result) - txt.nl() - txt.print("-.5 was expected\n\n") ; on C64: -1.1 is printed :( - -; result = value2*one + value1*one ; swapped operands around, now it's suddenly fine on C64... -; floats.print_f(result) -; txt.nl() -; txt.print("-.5 was expected\n\n") ; on C64: correct value is printed - - -; value1 = 0.8 -; value2 = 0.3 -; result = value1*one + value2*one -; floats.print_f(result) -; txt.nl() -; txt.print("1.1 was expected\n\n") ; on C64: correct value is printed -; -; printFAC1() -; printFAC1() -; printFAC1() - - repeat { - } - } - - sub printFAC1() { - ubyte[20] fac_save - sys.memcopy($60, fac_save, sizeof(fac_save)) - txt.print("fac1=") - %asm {{ - jsr floats.FOUT ; fac1 to string in A/Y - sta P8ZP_SCRATCH_W1 - sty P8ZP_SCRATCH_W1+1 - ldy #0 -- lda (P8ZP_SCRATCH_W1),y - beq + - jsr cbm.CHROUT - iny - bne - -+ }} - txt.nl() - sys.memcopy(fac_save, $60, sizeof(fac_save)) - } -}