SamCoVT / TaliForth2

A Subroutine Threaded Code (STC) ANSI-like Forth for the 65c02
Other
29 stars 5 forks source link

Reduce code/cycles for user+native code; clarify some comments #41

Closed patricksurry closed 6 months ago

patricksurry commented 6 months ago

A couple of minor things I noticed during code-browsing that might be helpful for other readers.

The only code change is for evaluating the forth_words/user_words from ROM. Both files are included as binary sequentially, and orphisbin adds whitespace to the end so (a) we can include both files with a single evaluate, and (b) in either case we don't need to do sbc calculations at run time, the assembler can do that for us.

patricksurry commented 6 months ago

it looks like my editor also removed some random occurrences of trailing whitespace

SamCoVT commented 6 months ago

That seems reasonable, and I'm always a fan of saving a few bytes.

I see you caught some typos, and I can see a couple more and a few other minor things to fix up here. Either you can fix them or I'll get them after merging this: definitions.asm:80 - change "User Variables" to "RAM System Variables" to help line up with the changes to the memory map diagram in the manual. native_words.asm:77 - spelling of evaluate native_words.asm:242 - spelling of initial

In preparation for merging this, can I get you to run make tests (the slow one) and push the results. I'm assuming there will be no errors, but I usually also check the diff on tests/results.txt, at the end with the cycle counts, to make sure no words take significantly longer than they used to. They will increase/decrease by a bit just due to where stuff crosses page boundaries, but nothing should be getting significantly longer/shorter. I'll double check it before merging.

I'm not concerned about the whitespace removed from the ends of lines. There is only one file where trailing whitespace needs to remain, and that's the cycle count tests in tests/cycles.fs (it makes all the results line up).

patricksurry commented 6 months ago

fixed the typos and added tests. cold drops 30 odd bytes, but surprising number of tests got v slightly longer cycle count: is that because one fewer evaluate?

SamCoVT commented 6 months ago

This looks good. Yes, just shuffling things by a byte or two is enough to change which words cross boundaries for things like branches and accessing data. Fortunately, we only support the 65C02 which handles this automatically - so we just get a penalty of an extra cycle when crossing a boundry with one of the indexed modes or a branch.

A slight change is normal. Words that loop a bunch of times (like for lookups in the dictionary) can change by a larger number of cycles, but for many words it's just a cycle or two. The ones with the large cycle count have to traverse through the entire dictionary (like the test for evaluate passing it the string with the 5 in it - that has to look through the entire dictionary before converting the 5 to a number), so those will change by more.

I see you already merged my previous merge, so I think this PR is all set to merge into master-64tass.