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

Support the x86 carry flag CF #30

Closed
akkartik opened this issue May 10, 2019 · 0 comments · Fixed by #31
Closed

Support the x86 carry flag CF #30

akkartik opened this issue May 10, 2019 · 0 comments · Fixed by #31

Comments

@akkartik
Copy link
Owner

akkartik commented May 10, 2019

When I selected the x86 instructions SubX would support I excluded the carry flag on the basis that higher-level languages didn't really provide access to it. However, it turns out it is needed for comparing unsigned integers. The current opcodes for jumping on lesser/greater rely on the sign flag which is set purely by treating comparison operands as signed integers. When comparing unsigned integers (say two addresses), we need to check the carry flag.

http://unixwiz.net/techtips/x86-jumps.html makes this obvious, whereas the doc I've been consulting so far (https://c9x.me/x86/html/file_module_x86_id_146.html) refers to the distinction in only a single sentence:

The terms "less" and "greater" are used for comparisons of signed integers and the terms "above" and "below" are used for unsigned integers.

akkartik added a commit that referenced this issue May 12, 2019
Tests failing.

This approach seems wrong. I'm not sure even the tests are correct. Also,
some open questions:

1. Should setting the overflow flag always set the carry flag?
2. Should the carry flag only be set on add/subtract/compare, or by all
arithmetic ops?
3. Had to turn off the -ftrapv flag in `build`. Is there a way to detect
overflow without actually causing overflow?

Once we start setting CF correctly we have to implement jump above/below
instructions (8- and 32-bit displacement variants).

#30
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 a pull request may close this issue.

1 participant