RRA not working properly

[marko-pi]

Observe the script that checks the most significant bit of the first parameter

lda p0 and 2147483648 sta p1 rra 31 sta p2

if you send(0xFFFFFFFF), you get (-1, -2147483648, -1)

OK, I understand that accumulator A is treated as a signed number, so 0xFFFFFFFF is -1 and 0x80000000 is -2147483648. However, if you rotate 0x80000000 (0b10000000000000000000000000000000) right 31 times, you should get 0x00000001 (0b00000000000000000000000000000001) and not 0xFFFFFFFF (0x11111111111111111111111111111111).

Now observe this script

lda p0 and 2147483648 sta p1 rra 31 sta p2 lda p0 and 1073741824 sta p3 rra 30 sta p4 lda p0 rra 31 sta p5 and 1 sta p6

if you send one parameter, (0xFFFFFFFF), you get (-1, -2147483648, -1, 1073741824, 1, -1, 1, 0, 0, 0)

The second part shows that checking the second most significant bit returns the correct number. The third part is workaround to avoid the bug. OK, I spent one day finding the error and I found workaround, but correcting the bug will save someone else's valuable time.

Plus, RRA is NOT rotate according to Assembler terminology. It is actually shift.

[joan2937]

Yes, this is a fault with the code. I was told about this some time ago but decided to do nothing about it at that time. My concern now and at the time was that some scripts may be depending on the incorrect behaviour.

What do you think @guymcswain ? Should it be fixed?

[marko-pi]

I have reported problems with rotate to you several months ago, so perhaps you are thinking of me.

Also, there is a problem with one of conditional jumps, I can check again which one.

[joan2937]

Thanks @marko-pi

It would be useful if you raised any other issues you have with the script command set in this issue.

(I did look for your e-mail but couldn't locate it after a quick search)

[marko-pi]

I will provide more information on other bugs in following days. Shall I open new issues or put everything here?

[joan2937]

If they are all on the script command set I would raise them here. They will probably be accepted or rejected as a batch.

If they are non-related issues then please start a new issue or issues.

[guymcswain]

My concern now and at the time was that some scripts may be depending on the incorrect behaviour.
What do you think @guymcswain ? Should it be fixed?

I think the practice we should follow is to fix an API if it does not adhere to the published specification. Likewise, it is unreasonable for users to rely on behavior that is not specified in the documentation for the API. I would only take exception to this rule if there were a significant number of users depending on the 'wrong' behavior. In that case, we could revert the fix but document the 'de-facto' behavior of the API. This particular issue would then become a request for a new API. This seems to me to be low probability so I'd advocate fix.

@marko-pi , if you submit a PR with a fix along with test cases it would expedite getting it into a release this year as I have no experience with the script APIs and the issues backlog will take precedence.

[marko-pi]

OK, I checked the jumps, and they are OK. If I remember right I reported that JP (jump positive) was jumping on zero, but after that you corrected the description of the command to "if (F>=0) PC=L", which is OK.

So please could you correct RRA and RLA? I will report the problem with script status in another thread.