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

PDP11: fix disassembly of reg,src instructions #331

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

pkoning2
Copy link
Member

This corrects the disassembly of MUL, DIV, ASH, and ASHC so it works correctly when the register is not R0.

This corrects the disassembly of MUL, DIV, ASH, and ASHC so it works
correctly when the register is not R0.
@pkoning2 pkoning2 merged commit c077c22 into open-simh:master Dec 18, 2023
18 checks passed
@al20878
Copy link
Contributor

al20878 commented Jan 30, 2024

Hi Paul,
I'm trying to figure out your change, but I must be missing something...
The masks array is used only in instruction disassembling, and is indexed with the instruction class number.
The class number is from the range 0..19 (20 possible values), but the masks array has 4x6 = 24 entries.
Why is the discrepancy?
Thanks!

@pkoning2
Copy link
Member Author

How did you get 24 entries? I count 20. Remember that when you look at the "changed files" display it shows you the difference, so the red line is a deleted line and the green line is its replacement.

@al20878
Copy link
Contributor

al20878 commented Jan 30, 2024

How did you get 24 entries?

static const int32 masks[] = {
0177777, 0177770, 0177700, 0177770,
0177700+I_D, 0177400+I_D, 0177700, 0177400,
0177400, 0177000, 0177000, 0177400,
0177400+I_D+I_L, 0170000, 0177777, 0177777,
0177000+I_D, 0177400+I_D, 0177700, 0177400,
0177000, 0177700+I_D, 0177400, 0177400+I_D+I_L
};

6 lines with 4 entries each, makes 24.

@pkoning2
Copy link
Member Author

Ok, I misread the "changed file" display as if it were the whole table.
The answer is "I have no idea". It was that way before; the fix doesn't change that. Perhaps @rms47 can tell us what happened.

@rms47
Copy link
Member

rms47 commented Jan 30, 2024

Table had duplicate entries for more than a year. It should be:

static const int32 masks[] = {
0177777, 0177770, 0177700, 0177770,
0177700+I_D, 0177400+I_D, 0177700, 0177400,
0177400, 0177000, 0177000, 0177400,
0177400+I_D+I_L, 0170000, 0177777, 0177777,
0177000, 0177400+I_D, 0177400, 0177400+I_D+I_L
};

The 24-entry table and this one are the same up until entry 15 (cond code set). The last four should be:

  1. SOPR - single operand, register - XOR
  2. FOPA - single flt single/double operand, faccum - fadd etc - moded for floating length
  3. SOPA - single operand, faccum - LDEXP
  4. SMDA - single int/long operand, faccum - moded for both integer length and floating length

I broke it at the end of 2022, when I added FOPA, SOPA, SMDA to account for all the src -> dst floating point cases:

31-Dec-22 RMS Floating loads are src,dst (nickd4)

and it got copied, verbatim, into V4.

@al20878
Copy link
Contributor

al20878 commented Jan 31, 2024

Thanks, @rms47!

SOPR - single operand, register - XOR

Although, XOR is a RSOP operation (as well as JSR), 10.
SOPR is for other arithmetic EIS ops, like MUL, DIV, ASH(C).

@pkoning2 , it looks like the version of the array that Bob says is correct does not agree with your patch...

Can you please review and re-patch?

@rms47
Copy link
Member

rms47 commented Feb 1, 2024

Thanks, @rms47!

SOPR - single operand, register - XOR

Although, XOR is a RSOP operation (as well as JSR), 10. SOPR is for other arithmetic EIS ops, like MUL, DIV, ASH(C).

True that. I checked that it worked for the EIS instructions and the fl ops that were added.

@al20878
Copy link
Contributor

al20878 commented Feb 21, 2024

@pkoning2 should I open another issue for this to be fixed?

Thanks!

@al20878
Copy link
Contributor

al20878 commented Feb 21, 2024

@rms47 : I checked http://simh.trailing-edge.com/sources/current/PDP11/ and pdp11_sys.c is still not fixed there

static const int32 masks[] = {
0177777, 0177770, 0177700, 0177770,
0177700+I_D, 0177400+I_D, 0177700, 0177400,
0177400, 0177000, 0177000, 0177400,
0177400+I_D+I_L, 0170000, 0177777, 0177777,
0177700+I_D, 0177400+I_D, 0177700, 0177400,           // <== This line should be deleted, erroneous copy of line 2
0177000, 0177700+I_D, 0177400, 0177400+I_D+I_L
};

@rms47
Copy link
Member

rms47 commented Feb 22, 2024

I just updated the whole directory. In addition to this fix, there are several interesting fixes to the Sigma, based on Ken Rector's ongoing work with diagnostics and exercisers.

al20878 added a commit to al20878/simh that referenced this pull request Jun 19, 2024
As discussed in PR#331, the fix needed more fix with the proper
masks array.  This patch completes the fix.
al20878 added a commit to al20878/simh that referenced this pull request Jul 23, 2024
As discussed in PR#331, the fix needed more fix with the proper
masks array.  This patch completes the fix.
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.

None yet

3 participants