search

SamCoVT / TaliForth2

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

Extract and refactor some shared code #67

Closed patricksurry closed 7 months ago

patricksurry commented 7 months ago

Saves 100 bytes or so and avoids duplication; equivalent performance

patricksurry commented 7 months ago

I meant that the code assumes n < 256 even tho it's passed on the stack as a 16bit word. I can clarify

On Wed, Apr 17, 2024 at 9:27 AM SamCoVT @.***> wrote:

@.**** commented on this pull request.

In taliforth.asm https://github.com/SamCoVT/TaliForth2/pull/67#discussion_r1568845294:

@@ -268,6 +268,108 @@ _nibble_to_ascii:

             rts

+ +find_header_name:

  • ; """Given <266 char string ( addr n -- )

I'm assuming you wanted "256" here

— Reply to this email directly, view it on GitHub https://github.com/SamCoVT/TaliForth2/pull/67#pullrequestreview-2006048271, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABA5MKVOE5TSMP2LILYEJZ3Y5Z2DXAVCNFSM6AAAAABGLFCKOKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMBWGA2DQMRXGE . You are receiving this because you authored the thread.Message ID: @.***>

SamCoVT commented 7 months ago

This looks really good. I'm not surprised there was some low-hanging savings here, as wordlists are a recent addition to Tali and they were "tacked on" rather than fully integrated from the start. There may be more optimizations available here, but I think you've gotten most of it already in this PR.

SamCoVT commented 7 months ago

I can probably review and merge this PR this coming weekend. You're welcome to push any other changes/tweaks you find in the meantime.

SamCoVT commented 7 months ago

I thought your original comment was clear enough - it just needed "256" instead of "266". It looks good in your recent commit.

patricksurry commented 7 months ago

ha, my brain still didn't see the doubled six, i was focused on the < sign

On Wed, Apr 17, 2024 at 9:43 AM SamCoVT @.***> wrote:

I thought your original comment was clear enough - it just needed "256" instead of "266". It looks good in your recent commit.

— Reply to this email directly, view it on GitHub https://github.com/SamCoVT/TaliForth2/pull/67#issuecomment-2061294777, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABA5MKVDSOJBAZHH3SY47SLY5Z365AVCNFSM6AAAAABGLFCKOKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRRGI4TINZXG4 . You are receiving this because you authored the thread.Message ID: @.***>

patricksurry commented 7 months ago

i'll stop messing with this branch and once we finalize I'll look at splitting up the native_words.asm