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

Add CMP Opcode #109

Merged
merged 18 commits into from
Jul 19, 2018
Merged

Add CMP Opcode #109

merged 18 commits into from
Jul 19, 2018

Conversation

Chris-Johnston
Copy link
Owner

@Chris-Johnston Chris-Johnston commented Jun 14, 2018

Adds the CMP Opcode which is responsible for setting all of the CCR bits.

Fixes #25 .

@Chris-Johnston Chris-Johnston added WIP Work in progress. opcode Add a new instruction for assembling and simulation labels Jun 14, 2018
@bpas247
Copy link
Collaborator

bpas247 commented Jun 14, 2018

So I was looking through PR #86 and I noticed that @AdamuKaapan has already implemented the CMP OPCODE in that PR. Is this the same implementation? If not, then how should we go about merging everyone's work?

@Chris-Johnston
Copy link
Owner Author

I had forgotten about that branch, since it had been failing the build for a while. Also, since PR #104 changed a lot of the way that opcodes are written, it may just be easier to re-do some of that work.

@bpas247
Copy link
Collaborator

bpas247 commented Jun 17, 2018

Could we rebase PR #86 with PR #104 and refactor it?

@Chris-Johnston
Copy link
Owner Author

To be honest, that sounds like more effort than it's worth. A lot of it would have to be rewritten anyways.

@bpas247
Copy link
Collaborator

bpas247 commented Jun 17, 2018

Then would it be best to close PR #86 for now?

@Chris-Johnston
Copy link
Owner Author

To be honest, that sounds like more effort than it's worth. A lot of it would have to be rewritten anyways.

if src_val.get_msb(self.size) is True:
if dest_val.get_msb(self.size) is False:
if raw_total > 0 and raw_total & 0x80000000 > 0:
overflow = True
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic was taken from CMPI, which was taken from SUB and SUBQ. However, I used new logic to identify the overflow bit for SUB and SUBQ. I would recommend checking it out and seeing if it might work better than this implementation.

@bpas247
Copy link
Collaborator

bpas247 commented Jun 30, 2018

Is there anything else that should be done for this PR?

@Chris-Johnston Chris-Johnston changed the title [WIP] Add CMP Opcode Add CMP Opcode Jul 12, 2018
@Chris-Johnston
Copy link
Owner Author

Looks good! Sorry for the delay in PR review.

@Chris-Johnston Chris-Johnston merged commit 2d883aa into master Jul 19, 2018
@Chris-Johnston Chris-Johnston deleted the opcode-cmp branch July 19, 2018 06:28
@Chris-Johnston Chris-Johnston removed the WIP Work in progress. label Jul 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
opcode Add a new instruction for assembling and simulation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants