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

Altair 8800 DAA instruction doesn't adjust for carry bit #433

Open
jaredkrinke opened this issue Nov 8, 2024 · 4 comments
Open

Altair 8800 DAA instruction doesn't adjust for carry bit #433

jaredkrinke opened this issue Nov 8, 2024 · 4 comments

Comments

@jaredkrinke
Copy link

  • Context

Disclaimer: I don't have an actual Altair 8800 to test against, so it's possible there's a misunderstanding on my part and/or a bug in the other emulator I'm testing against.

On the Altair 8800 (Intel 8080), for the DAA instruction, the 8080 Programmers Manual says:

If the most significant four bits of the accumulator now represent a number greater than 9, or if the normal carry bit is equal to one, the most significant four bits of the accumulator are incremented by six.

It looks like altair_cpu.c only checks the auxiliary carry flag and not the normal carry flag. To test this, I ran the following program (which adds 0x81 to 0x81 and runs DAA, which should result in 0x62 at address 0x40, since 81 + 81 = 162), but the result at 0x40 is 0x02 instead of 0x62. A different Altair simulator shows 0x62 as the result.

	mvi a, 81h	; Add 0x81 to 0x81, both binary-coded decimal
	adi 81h
	daa		; Correct for BCD; result should be 0x62, with carry
	sta result
	hlt		; Result at 0100 should be 0x62

	org 100o
result:	db 0
  • the output of "sim> SHOW VERSION" while running the simulator which is having the issue

Altair 8800 simulator V3.8-1 [32b data, 32b addresses, no Ethernet]

  • how you built the simulator or that you're using prebuilt binaries

Prebuilt x86-64 Debian Bookworm binaries

  • the simulator configuration file (or commands) which were used when the problem occurred.

No custom configuration file; GitHub won't let me attach the compiled .bin file, so here it is in octal:

076 201 306 201 047 062 100 000 166 (with a zero at address 0x40/octal 0100)

  • the expected behavior and the actual behavior

Output of running the above program and checking the result at octal 0100:

sim> load daa.bin
65 Bytes loaded.
sim> run

HALT instruction, PC: 000010 (HLT)
sim> e 100
100:    002
  • you may also need to provide specific pointers to data files that may be necessary to demonstrate the problem

See assembly and octal compiled program above.

@psco
Copy link
Contributor

psco commented Nov 8, 2024

You are right. The DAA instruction is not correctly implemented in the Altair simulator. I suggest to use the AltairZ80 simulator instead. It also can simulate an Altair 8800 and has a correct implementation of the DAA instruction for both 8080 and Z80.

@jaredkrinke
Copy link
Author

I suggest to use the AltairZ80 simulator instead. It also can simulate an Altair 8800 and has a correct implementation of the DAA instruction for both 8080 and Z80.

Thanks! That seems to work for my code.

For the record, it looks like the plain old altair binary also doesn't set the auxiliary carry flag on addition, so my trivial attempt at fixing this issue (just checking the regular carry flag instead of the auxiliary carry flag for the upper binary coded digit) doesn't completely solve binary-coded arithmetic.

@markpizz
Copy link
Contributor

markpizz commented Nov 9, 2024

Now that you've identified a clear bug, you can suggest a specific fix. If you're up for that, please work against the latest code here in the master branch. If you're familiar with git, feel free to submit a pull request. Otherwise, just identify the appropriate code change here and I'll be glad to put the change into the system here with you as the author.

@jaredkrinke
Copy link
Author

jaredkrinke commented Nov 9, 2024

Unfortunately, the fix I had in mind was just to check the carry flag, but it looks like it is insufficient to get DAA completely working. I think the remaining issue is that the auxiliary carry flag isn't getting set correctly during ADD/ADI. That's as far as I've gotten.

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

No branches or pull requests

3 participants