Lol, the -2nu compile option works! No warning and the module plays nicely.
EDIT: ALso checked again with newest master flexspin build using -2 compile option - no change in warning/outcome:
Propeller Spin/PASM Compiler 'FlexSpin' (c) 2011-2022 Total Spectrum Software Inc.
Version 5.9.11-beta-v5.9.10-34-gcad092d5 Compiled on: May 18 2022
main.spin2
|-simpleSound.spin2
|-|-reSound.spin2
|-|-trackerPlayer.spin2
warning: Internal warning, found two GETQX during constant propagate??
warning: Internal warning, found two GETQX during constant propagate??
main.p2asm
Done.
Program size is 462064 bytes
Some of the CORDIC optimizations were being applied to inline assembly, when they really shouldn't have been. Changing that that gets rid of the warning, and I've checked it in. I haven't had a chance to see if it fixes the runtime problem.
@Wuerfel_21 said:
I apologize for putting all those bugs in.
Au contraire, I'm the one who reviewed the code and put it into the repository, so if there's any blame it's mine. Bugs happen, but it's great that we have a community that helps to find and fix them (and to improve flexspin in other ways, as you've done so much Ada!).
Darn, not fully resolved. No warnings now with -2 compile option but still get just low fuzzy noise on playback ... looking at changes in .lst there's just one ... newer version:
Huh, doing some fiddling by inserting some muldiv64() lines and got another of the prior warning:
Propeller Spin/PASM Compiler 'FlexSpin' (c) 2011-2022 Total Spectrum Software Inc.
Version 5.9.11-beta-v5.9.10-38-gf23ce1a9 Compiled on: May 19 2022
main.spin2
|-simpleSound.spin2
|-|-reSound.spin2
|-|-trackerPlayer.spin2
warning: Internal warning, found two GETQX during constant propagate??
main.p2asm
Done.
I can see QDIV results are not retrieved below:
01754 | ' ' Calculate LUTs according to the parameters given
01754 | ' repeat i from MINIMUM_PERIOD to MAXIMUM_PERIOD
01754 71 BC 05 F6 | mov arg01, #113
01758 04 98 05 F1 | add ptr__trackerPlayer_dat__, #4
0175c | LR__0046
0175c DE BE 01 FD | qmul arg02, arg01
01760 | ' ' frequencyLut[i - MINIMUM_PERIOD] := calculateAccumulatorFrequency(mixingFrequency, i)
01760 | ' frequencyLut[i - MINIMUM_PERIOD] := muldiv64( 3546894, 65536, mixingFrequency * i)
01760 18 C0 61 FD | getqx arg03
01764 28 6C 64 FD | setq #54
01768 00 87 8F FF
0176c E0 00 18 FD | qdiv ##521011200, arg03
01770 DE C4 01 F6 | mov arg05, arg01
01774 71 C4 85 F1 | sub arg05, #113
01778 02 C4 65 F0 | shl arg05, #2
0177c CC C4 01 F1 | add arg05, ptr__trackerPlayer_dat__
01780 00 87 8F FF
01784 E2 00 68 FC | wrlong ##521011200, arg05
01788 01 BC 05 F1 | add arg01, #1
0178c 01 00 00 FF
01790 59 BD 15 F2 | cmp arg01, ##857 wc
01794 C4 FF 9F CD | if_b jmp #LR__0046
Thanks, Evan. I think the problem may have been that once we started doing the cordic constant propagation, it kept going, even if a new (non-constant) cordic command is issued. I've checked in a fix for that.
Must be more to it, still the same low buzzing ... well maybe not completely the same. There's also some squealing and very high tones and step changes in the buzzing. Nah, it's the same.
EDIT: I've still got a couple of old copies of flexspin built 30 Mar and 11 Apr. The 30 Mar version is good, no warnings and plays audio correctly. The 11 Apr version is not.
BTW: The latest build has no warnings when compiling the simpleSound demo.
Propeller Spin/PASM Compiler 'FlexSpin' (c) 2011-2022 Total Spectrum Software Inc.
Version 5.9.10-beta-v5.9.9-93-ge37a63f5 Compiled on: Mar 30 2022
main.spin2
|-simpleSound.spin2
|-|-reSound.spin2
|-|-trackerPlayer.spin2
main.p2asm
Done.
Propeller Spin/PASM Compiler 'FlexSpin' (c) 2011-2022 Total Spectrum Software Inc.
Version 5.9.10-beta-v5.9.9-155-g49b31c5d Compiled on: Apr 11 2022
main.spin2
|-simpleSound.spin2
|-|-reSound.spin2
|-|-trackerPlayer.spin2
warning: Internal warning, found two GETQX during constant propagate??
main.p2asm
Done.
511a7758a9f1fbe2b2e95ef36e9d3b9fb9110002 is the first bad commit
commit 511a7758a9f1fbe2b2e95ef36e9d3b9fb9110002
Author: Ada Gottensträter <nunuu@gmx.net>
Date: Fri Apr 1 20:55:59 2022 +0200
Fix constant propagation a bit
backends/asm/optimize_ir.c | 49 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 43 insertions(+), 6 deletions(-)
511a7758a9f1fbe2b2e95ef36e9d3b9fb9110002 is the first bad commit
commit 511a7758a9f1fbe2b2e95ef36e9d3b9fb9110002
Author: Ada Gottensträter <nunuu@gmx.net>
Date: Fri Apr 1 20:55:59 2022 +0200
Fix constant propagation a bit
backends/asm/optimize_ir.c | 49 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 43 insertions(+), 6 deletions(-)
Seem innocuous enough...
Try comparing the generated asm before and after the commit. That should narrow it down (I could/should probably do that myself, for I broke it, but oh well)
@Wuerfel_21 said:
Try comparing the generated asm before and after the commit. That should narrow it down (I could/should probably do that myself, for I broke it, but oh well)
Here's both with mod data trimmed off.
"main-v5.9.9-95.lst" is with commit e0e8ce0b562b2c5032006d76e883b69a9d5e7430
"main-v5.9.9-96.lst" is with commit 511a7758a9f1fbe2b2e95ef36e9d3b9fb9110002
EDIT: Added "main-v5.9.10-40.lst" file with current master branch
Thanks @evanh and @Wuerfel_21 for your help tracking this down. I think it was a problem in the ReadWrite optimization (so a separate bug from the CORDIC issue fixed earlier) and I've checked in a fix.
Gee, must have been a corner case to only affect simpleSounds/reSound like that. I'm curious now to know what was being tripped up.
EDIT: Huh, only a single instruction extra in the fixed version. No idea why this case is special though, the source code looks rather ordinary really. EDIT2: Ah, it's the double divide isn't it. muldiva_ needs reloaded with same systemClock but the optimiser thought it didn't.
I wrote a longer topic with an example in the Basic section. Here is the short report:
There seems to be inconsistency in directories where the compiler searches for files declared in Basic's "class using"
If "class using" is used "normally", somewhere at the start of the program, it searches for the file in the project directory (as expected).
However, If "class using" is used inside a class declaration, then it seems the compiler searches for the file in its starting directory instead of a project directory, and then, if they are different, generates "file not found" error.
I have to write this to the point it can be tested. I have now a class which, while testing, showed a bug in the video driver, so I have to debug the driver, then the class.
@pik33 said:
I have to write this to the point it can be tested. I have now a class which, while testing, showed a bug in the video driver, so I have to debug the driver, then the class.
Works for me!
As Eric will tell you, I’m really more of a code bludgeoner than a beta tester, but I have fun and find a few bugs in the process. 🤣
Eric,
Struck something that doesn't even make sense to me. It looks like there is a flaw in hardware of immediate branching. The deltas within cogRAM seem to work as byte scaled (which can't cover all of cogRAM) rather than longword scaled! I.e.: The below runs correctly:
Both are compiled as pure pasm2 code with Flexspin Version 5.9.11-beta-v5.9.10-41-g34a1753f Compiled on: May 21 2022. Second variant occurs when using ##D, but #S is the one with the problem.
Oops, doh! Didn't actually read the opcode encoding of the first variant. Of course it's byte granularity because it's 20-bit address encoding within that instruction.
Okay, so the second variant still don't work ... okay, looks like it is a bug in Flexspin: I can now see the offset is one too far. And to prove that I added NOP as first instruction of #waitms routine and it then works correctly. Guess there is a compiling error with the executing address (PC) at the CALLPA because of the prefixed AUGD.
@evanh : The calculation is using longword addresses rather than byte scaled -- the callpa source address is 0x68 (according to the listing) which correspoinds to 0x8b - 0x23. The problem is that it's using the wrong base address (the address of the aug instruction rather than the address of the callpa instruction). That's a flexspin bug and I will try to fix it. In the meantime, the work-around is to not use ## with callpa. If you really want to you could insert an explicit augd, I think that would work.
Yeah, I got there when I took a second look. ... Then completed the desired 400+ MHz testing without tripping on that issue. Maxed out at 425 MHz without hubRAM errors at -10 oC.
Comments
Lol, the -2nu compile option works! No warning and the module plays nicely.
EDIT: ALso checked again with newest master flexspin build using -2 compile option - no change in warning/outcome:
Some of the CORDIC optimizations were being applied to inline assembly, when they really shouldn't have been. Changing that that gets rid of the warning, and I've checked it in. I haven't had a chance to see if it fixes the runtime problem.
I apologize for putting all those bugs in.
Au contraire, I'm the one who reviewed the code and put it into the repository, so if there's any blame it's mine. Bugs happen, but it's great that we have a community that helps to find and fix them (and to improve flexspin in other ways, as you've done so much Ada!).
Darn, not fully resolved. No warnings now with -2 compile option but still get just low fuzzy noise on playback ... looking at changes in .lst there's just one ... newer version:
I would imagine there is meant to be a RET in the Fcached code. LR__0163 in the wrong place?
EDIT: Okay, not that I guess. Compiling a different program with inline pasm, producing equivalent fcache labelling config, but works fine.
Huh, doing some fiddling by inserting some
muldiv64()lines and got another of the prior warning:I can see QDIV results are not retrieved below:
Thanks, Evan. I think the problem may have been that once we started doing the cordic constant propagation, it kept going, even if a new (non-constant) cordic command is issued. I've checked in a fix for that.
Must be more to it, still the same low buzzing ... well maybe not completely the same. There's also some squealing and very high tones and step changes in the buzzing. Nah, it's the same.
EDIT: I've still got a couple of old copies of flexspin built 30 Mar and 11 Apr. The 30 Mar version is good, no warnings and plays audio correctly. The 11 Apr version is not.
BTW: The latest build has no warnings when compiling the simpleSound demo.
Try git bisect, that'll find the problem commit.
Unrelatedly, what could be the cause of Error code 7 (ENOMEM) from fopen? Is the default heap too small for two files at once?
I'm clueless even on how to roll back let alone use of bisect. Not quite so clueless now.
EDIT: Okay, did a search and found out bisect handles it all. Nice! Got all the hints I needed from https://www.metaltoad.com/blog/beginners-guide-git-bisect-process-elimination
511a7758a9f1fbe2b2e95ef36e9d3b9fb9110002 is the first bad commit commit 511a7758a9f1fbe2b2e95ef36e9d3b9fb9110002 Author: Ada Gottensträter <nunuu@gmx.net> Date: Fri Apr 1 20:55:59 2022 +0200 Fix constant propagation a bit backends/asm/optimize_ir.c | 49 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 6 deletions(-)Seem innocuous enough...
Try comparing the generated asm before and after the commit. That should narrow it down (I could/should probably do that myself, for I broke it, but oh well)
Seems I forgot to close a file earlier, but that just means the default heap is too small for three files at once... two are ok ig
Here's both with mod data trimmed off.
"main-v5.9.9-95.lst" is with commit e0e8ce0b562b2c5032006d76e883b69a9d5e7430
"main-v5.9.9-96.lst" is with commit 511a7758a9f1fbe2b2e95ef36e9d3b9fb9110002
EDIT: Added "main-v5.9.10-40.lst" file with current master branch
You want the .p2asm files. listings have too much non-semantic noise that makes it obnoxious to compare them
Thanks @evanh and @Wuerfel_21 for your help tracking this down. I think it was a problem in the ReadWrite optimization (so a separate bug from the CORDIC issue fixed earlier) and I've checked in a fix.
Yep, thanks Eric, works now. ReadWrite of ... Cordic ops?
No, actually rdlong/wrlong. There were multiple bugs, not all CORDIC related
. But I think they're fixed now.
Gee, must have been a corner case to only affect simpleSounds/reSound like that. I'm curious now to know what was being tripped up.
EDIT: Huh, only a single instruction extra in the fixed version. No idea why this case is special though, the source code looks rather ordinary really. EDIT2: Ah, it's the double divide isn't it.
muldiva_needs reloaded with samesystemClockbut the optimiser thought it didn't.The bad (Compiled with Flexspin v5.9.10-40):
' if reSoundHqMode if_e jmp #LR__0045 ' mixingFrequency := systemClock / ((systemClock / mixingFrequency) & $FF00) rdlong muldiva_, objptr mov muldivb_, arg02 call #divide_ and muldivb_, ##65280 call #divide_ mov arg02, muldivb_ LR__0045The good (Compiled with Flexspin v5.9.10-41):
' if reSoundHqMode if_e jmp #LR__0045 ' mixingFrequency := systemClock / ((systemClock / mixingFrequency) & $FF00) rdlong muldiva_, objptr mov muldivb_, arg02 call #divide_ and muldivb_, ##65280 rdlong muldiva_, objptr call #divide_ mov arg02, muldivb_ LR__0045I wrote a longer topic with an example in the Basic section. Here is the short report:
There seems to be inconsistency in directories where the compiler searches for files declared in Basic's "class using"
If "class using" is used "normally", somewhere at the start of the program, it searches for the file in the project directory (as expected).
However, If "class using" is used inside a class declaration, then it seems the compiler searches for the file in its starting directory instead of a project directory, and then, if they are different, generates "file not found" error.
Thanks, @pik33 . Nested classes in BASIC haven't really been tested very well. I'll look into it.
A heavy test of Basic classes is now started as I started to write a GUI for a P2 using them
Silly question: any reason > @pik33 said:
If you need a beta tester, consider this an offer!
I have to write this to the point it can be tested. I have now a class which, while testing, showed a bug in the video driver, so I have to debug the driver, then the class.
Works for me!
As Eric will tell you, I’m really more of a code bludgeoner than a beta tester, but I have fun and find a few bugs in the process. 🤣
This is not a beta. This is not even alpha yet. But this is a moving window using a Basic class : https://gitlab.com/pik33/P2-retromachine/-/blob/main/Propeller/Videodriver_develop/videotest2.bas. Needs PSRAM. I don't know yet if it will work with a standard EC32 configuration as I am using 4-bit PSRAM driver now. To not contaminate this topic: the driver's topic is https://forums.parallax.com/discussion/173228/a-display-list-is-fun-0-50a-beta-0-07g-psram-command-list-used#latest
Eric,
Struck something that doesn't even make sense to me. It looks like there is a flaw in hardware of immediate branching. The deltas within cogRAM seem to work as byte scaled (which can't cover all of cogRAM) rather than longword scaled! I.e.: The below runs correctly:
but this doesn't:
Both are compiled as pure pasm2 code with Flexspin
Version 5.9.11-beta-v5.9.10-41-g34a1753f Compiled on: May 21 2022. Second variant occurs when using ##D, but #S is the one with the problem.Oops, doh! Didn't actually read the opcode encoding of the first variant. Of course it's byte granularity because it's 20-bit address encoding within that instruction.
Okay, so the second variant still don't work ... okay, looks like it is a bug in Flexspin: I can now see the offset is one too far. And to prove that I added NOP as first instruction of #waitms routine and it then works correctly. Guess there is a compiling error with the executing address (PC) at the CALLPA because of the prefixed AUGD.
@evanh : The calculation is using longword addresses rather than byte scaled -- the callpa source address is 0x68 (according to the listing) which correspoinds to 0x8b - 0x23. The problem is that it's using the wrong base address (the address of the aug instruction rather than the address of the callpa instruction). That's a flexspin bug and I will try to fix it. In the meantime, the work-around is to not use ## with callpa. If you really want to you could insert an explicit augd, I think that would work.
@ersmith It looks like SHR, SHL, <<, and >> are broken in FlexBASIC Version 5.9.11.
This code:
option explicit dim a as ulong a = &h55555555 a << 1 print athrows this error:
BTW: This is not sensitive to optimization level, etc. It just sees a shift anywhere in the code, and it goes instantly kerplonk.