Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String op improvements #141

Open
wants to merge 2 commits into
base: master-64tass
Choose a base branch
from

Conversation

patricksurry
Copy link

Saves about 50 bytes overall. Simplifies CLEAVE (faster, smaller and avoids input manipulation). Shortens -LEADING/TRAILING: this slows it down a little after removing some inlining, but also because I switched to the shared jsr is_whitespace instead of the specific cmp #AscSP. I'm not sure if that was intentional but seemed like it should be symmetric with LEADING and CLEAVE?

The overall test cycles increase somewhat because -TRAILING is used in various tests, but I imagine in practice the CLEAVE improvement might be more important? Equally I could just undo the -TRAILING changes.

Cycle counts with simple test:

word before after
s" a b c " CLEAVE 1225 487
s" txt " -LEADING 354 234
s" txt " -TRAILING 224 430*

*337 with cmp #AscSp instead of jsr is_whitespace.

cmp #AscSP
bne _done
; Move back to point at last character
jsr w_one_minus
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this inline slows down the loop; could undo

; While spaces are found,
; decrease the count on the data stack and repeat
lda (0,x)
jsr is_whitespace
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this also slows it down, but seems better than just cmp #' ' unless that was intentionally avoiding other whitespace?

@SamCoVT
Copy link
Owner

SamCoVT commented Oct 21, 2024

One of the differences is that -TRAILING is an ANS standard word, while the others are Tali specific. The standard specifically mentions spaces, which are elsewhare defined as BL and the value 0x20.
https://forth-standard.org/standard/string/MinusTRAILING

@patricksurry
Copy link
Author

ah, that makes sense. so I'll switch -TRAILING back to only check for spaces. Does it make sense for -LEADING to behave in a similar same (only strip spaces specifcally) or should it continue to strip other whitespace as well?

@SamCoVT
Copy link
Owner

SamCoVT commented Oct 29, 2024

I agree that -TRAILING should use only spaces, but I'm fine with the other words skipping any whitespace and I think your changes look fine for those words.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants