Optimize mult macro - Githubissues

joeldipops / gbz80-pseudoOps

Set of rgbasm macros and constants to facilitate readability for modern programmers

MIT License

8 stars 1 forks source link

Optimize mult macro #8

Closed ISSOtm closed 5 years ago

ISSOtm commented 5 years ago

Adapted from http://wikiti.brandonw.net/index.php?title=Z80_Routines:Math:Multiplication Now constant time, and uses less regs

joeldipops commented 5 years ago

This solution doesn't work out of the box, and doesn't quite line up with my interface principles, but don't disagree that the current implementation of mult is pretty shit.

I'll close the pull requests, but open a new issue in its stead.

ISSOtm commented 5 years ago

This solution doesn't work out of the box

How so? The macro works fine, compiles to working code... Where's the problem?

and doesn't quite line up with my interface principles

You don't state them, so I can't try to improve them.

Also, closing the issues without giving a chance to improve is pretty bad if you ask me.

joeldipops commented 5 years ago

As I said, I opened an issue to improve the macro. It's right here and includes the code you wrote. https://github.com/joeldipops/gbz80-pseudoOps/issues/11.

The code changed substantially since the request was opened and I'm not knowledgeable enough about git and github to simply merge the code straight from the PR.

joeldipops commented 5 years ago

I had another look at the macro, and as I thought at the time, it neither compiles nor works. Some issues

Syntax error where jr NC, .noAdd\@ doesn't match up with the .noAdd label missing the \@
No reference to \1 anywhere in the macro - I'm guessing the HIGH(\2) and LOW(\2) should be 1s instead.
Reference to DE which holds an arbitrary value at this point. Assume \1 instead.

With those fixes it does look like it's working now, but didn't out of the box. I want to take the time make sure I know how this works and fiddle with the interface if I can.