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

IBM 1130 Fix address computation overflow. #386

Closed
wants to merge 1 commit into from

Conversation

charlesUnixPro
Copy link
Contributor

In sim_instr(), the effective address is computed; for the case of TAG (index register addressing), the contents of
the specified index register is added to the effective address, but the result is not masked to 16 bits as per the
hardware functionality.

Adding a a 16 bit mask operation fixes the issue.

@markpizz
Copy link
Contributor

markpizz commented Jun 8, 2024

I have nothing to say about the technical merits of your suggested change. I'm sure you are right.

Meanwhile, the project conventions about the format of commit messages is:

affected-simulator-or-component: one-line summary of the change <= 80 in length
<blank-line-if-more-commit-message-is-appropriate>
<additional-commit-comments-less-than-80>

The commit message should contain all the relevant explanation for the change. The Pull Request comments might be the same, but the commit message should contain the full justification. The reason being is that the commits exist as part of the history in every copy/clone of the repo without any dependence on the specific Github instance.

So, for this Pull Request we'd like to see the commit message be:

IBM1130: Fix address computation overflow

In sim_instr(), the effective address is computed; for the case of TAG
(index register addressing), the contents of the specified index register
is added to the effective address, but the result is not masked to 16
bits as per the hardware functionality.

Adding a a 16 bit mask operation fixes the issue.


You can readily back out the most recent commit locally:
$ git reset --soft HEAD^
and the commit again with the appropriate commit message and then push out to github with the --force flag that will properly update the pull request.

In sim_instr(), the effective address is computed; for the case of TAG
(index register addressing), the contents of the specified index register
is added to the effective address, but the result is not masked to 16
bits as per the hardware functionality.

Adding a a 16 bit mask operation fixes the issue.
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 this pull request may close these issues.

2 participants