I decided to print off and review the code for v2.07 seeing as it's now functionally complete.
After completing the initial review, I've managed to shave off 108 bytes from the assembled module. I'm quite pleased with the reduction in size which comes from a simple re-organising of some code which allowed me to remove what was effectively duplicate redundant code which only executed in a particular branch. There is absolutely no impact on features or functions for this reduction in size.
I also found and fixed a minor bug thanks to Armalyser that would only occur under rare circumstances when a WE VIDC Enhancer is installed. Even then, it would still work but since it's been highlighted, I can't leave it there!
It all seems to be working as expected but I'll do some testing of the module over the weekend to make sure it's working fully before I release v2.08.
After that, I'm going to bury my head in ARM optimisation techniques to see if I can't squeeze more bytes out of the module.
Paul
Code review
Re: Code review
Sounds like you're, on a mission!
I was pondering doing something similar to ADFFS after I noticed extASM trying to expand LDR PC, <address> into two statements using PC as the operand!! I should probably look at the extASM source to fix it also.
Think there's around 2K of auto-expansion now, which makes me cringe! I have to be careful with that, as it may cause issues in the Vector handlers if any auto-expand.
I was pondering doing something similar to ADFFS after I noticed extASM trying to expand LDR PC, <address> into two statements using PC as the operand!! I should probably look at the extASM source to fix it also.
Think there's around 2K of auto-expansion now, which makes me cringe! I have to be careful with that, as it may cause issues in the Vector handlers if any auto-expand.
Re: Code review
I just know that as a first large ARM assembler project, I'll have done things in such a way as to make sure it works but it won't necessarily be the fastest/most efficient/smallest code way of doing things.
Optimising the code is stage two of my ARM assembler education
Paul
Optimising the code is stage two of my ARM assembler education
Paul
Re: Code review
Don't you just love code reviews. 
I've found three errors in code where old code had not been removed and although it wasn't doing anything harmful (or anything at all in one case), it was taking up space. So I've removed those pieces of code.
I've also found a couple of bugs which I've fixed so overall, I've still shaved off over 90 bytes of code from v2.07 modules size.
I've been thinking that my next step is to hive off the three VIDC clock setting code functions into separate files so that I can re-use the code as functions rather than repeating sections of code in two or three different places.
I'm thinking that if I stick the registers onto the stack then, re-prime the relevant registers to pass into the function by doing a Branch and Link. Once the function is done, I can then return and unload the preserved registers from the stack.
Hopefully, that should let me reduce the code size significantly. Does that sound reasonable?
Paul
I've found three errors in code where old code had not been removed and although it wasn't doing anything harmful (or anything at all in one case), it was taking up space. So I've removed those pieces of code.
I've also found a couple of bugs which I've fixed so overall, I've still shaved off over 90 bytes of code from v2.07 modules size.
I've been thinking that my next step is to hive off the three VIDC clock setting code functions into separate files so that I can re-use the code as functions rather than repeating sections of code in two or three different places.
I'm thinking that if I stick the registers onto the stack then, re-prime the relevant registers to pass into the function by doing a Branch and Link. Once the function is done, I can then return and unload the preserved registers from the stack.
Hopefully, that should let me reduce the code size significantly. Does that sound reasonable?
Paul
Re: Code review
Sounds sensible to me
Re: Code review
Ok, after what turned out to be minimal changes other than ripping out a couple of hundred lines of source code, I've managed to reduce the module to 6780 bytes from what was at it's peak 7320 bytes.
In making the changes I introduced a little bug in the change that meant that when an Aux IO based board was being used, going from MODE 31 to MODE 27 on RISC OS 2 didn't reset the clock to 24MHz.
My initial solution to this is simple. First, set the clock to 24MHz, then parse the mode lists and if a MODE matches, set the clock accordingly.
I could've implemented a "clockNotSetYet" variable that is defaulted to false and subsequently set to true when the clock setting code is called. After getting to the cleanup code, I could then check the "clockNotSetYet" value and if it's still false set the clock to 24MHz.
Both ways work, the way I've done it is a bit of a scatter gun approach as it calls the setClock routines a maximum of twice. If I implement the clockNotSetYet variable, it'll only every call the setClock routines once.
After describing all that, I'm going to implement the variable method as it'll be a tad quicker to run and it's neater and the current solution is effectively a quick hack at 2am to make it work... Hopefully, I should have a new version of the module for further testing for everyone at some point tomorrow.
Paul
In making the changes I introduced a little bug in the change that meant that when an Aux IO based board was being used, going from MODE 31 to MODE 27 on RISC OS 2 didn't reset the clock to 24MHz.
My initial solution to this is simple. First, set the clock to 24MHz, then parse the mode lists and if a MODE matches, set the clock accordingly.
I could've implemented a "clockNotSetYet" variable that is defaulted to false and subsequently set to true when the clock setting code is called. After getting to the cleanup code, I could then check the "clockNotSetYet" value and if it's still false set the clock to 24MHz.
Both ways work, the way I've done it is a bit of a scatter gun approach as it calls the setClock routines a maximum of twice. If I implement the clockNotSetYet variable, it'll only every call the setClock routines once.
After describing all that, I'm going to implement the variable method as it'll be a tad quicker to run and it's neater and the current solution is effectively a quick hack at 2am to make it work... Hopefully, I should have a new version of the module for further testing for everyone at some point tomorrow.
Paul