jkotlinski / durexforth

Modern C64 Forth
Other
230 stars 28 forks source link

added `compare` #520

Closed ekipan closed 1 year ago

ekipan commented 1 year ago

I believe it to be ANS Forth correct. I haven't looked at your testing rig but it'd be nice to use the official tests.

This is probably much better done in direct assembly though.

jkotlinski commented 1 year ago

Thanks for the contribution! Bonus points for testing!

The semi-official test suite for the STRING word set is here: https://github.com/gerryjackson/forth2012-test-suite/blob/dfaf2e662545c84e63d72f9a14d307a5db82fc44/src/stringtest.fth That could be brought in to be extra sure that it is standard compliant.

ekipan commented 1 year ago

Bleh, The tests worked in VICE when I tried them from v directly. Cycles again. I do not know what could be wrong.

jkotlinski commented 1 year ago

The build archived a screenshot from the test run: bild

ekipan commented 1 year ago

Probably needs to be in the Makefile? Gimme a sec.

ekipan commented 1 year ago

Whoops, maybe I should actually read this thing before committing.

jkotlinski commented 1 year ago

The word should probably be described in manual, in "Strings" section. Also, the addition should be mentioned in CHANGELOG.md

ekipan commented 1 year ago

Clearly the PR is half-baked. Would you prefer I retract for now?

jkotlinski commented 1 year ago

It is fine to keep it open. You can convert it to Draft if you like.

ekipan commented 1 year ago

(The force push was an extra blank line in tests.fs I deleted)

ekipan commented 1 year ago

Would you prefer file compare to be an optional include, as I have here, or try putting it into base? I heard the cartridge was tight on bytes.

ekipan commented 1 year ago

I wrote this word and thought "hey that seems useful" and submitted on a lark, but really memcmp should be assembler, and I'm sure it already exists in the interpreter somewhere so it would be better to reorganize and expose that, but that's more work than I'm able to do at my level. Thanks for humoring me though.

Whammo commented 1 year ago

Would you Open a Discussion, maybe in Show-and-tell, show your code and document it there? A straight memory compare is a useful thing and offers many opportunities for use-specific optimization. I am certain I would be grateful, and there might even be a half a dozen other people in the world interested.

jkotlinski commented 1 year ago

Hey! Sorry for not responding promptly. I had too much real-life things going on lately.

It is true, reuse value would be higher if the word was written efficiently in assembly. I would look at cc65 strcmp/memcmp for inspiration.

I do not think durexForth already has string matching code that is suitable for reuse. It's either too optimized (interpreter.asm) or not particularly good or fast (v.fs).