Skip to content

Fix for [sp+n] instruction length, DIV EXtension, EX evaluation order and option for shift behaviour#49

Open
orlof wants to merge 4 commits intoHerobrinesArmy:masterfrom
orlof:master
Open

Fix for [sp+n] instruction length, DIV EXtension, EX evaluation order and option for shift behaviour#49
orlof wants to merge 4 commits intoHerobrinesArmy:masterfrom
orlof:master

Conversation

@orlof
Copy link

@orlof orlof commented Mar 12, 2016

No description provided.

@orlof orlof changed the title fixed instruction length for sp+n Emulator fixes for [sp+n] length and ex evaluation order Mar 15, 2016
orlof and others added 2 commits March 15, 2016 13:39
@HerobrinesArmy
Copy link
Owner

Can you explain the reasons for each of the changes (are they corrections to incorrect emulation behaviour; if so, what was wrong, etc.)? Also, it looks like, in "fixed instruction length for sp+n", on line 235, you put "atype == 26" when it looks like you probably meant to put "btype == 26". Then again, I don't know what the change was for in the first place, so maybe it's supposed to be that way. :P

Also, my applause to you for diving into a bunch of mostly-undocumented code to try to improve it. I know that's always fun. :P

@orlof
Copy link
Author

orlof commented Apr 18, 2016

(edited to clarify text)

Here are my excuses... :)

Sorry for the low quality PR - I wasn't expecting anyone to process it as last commits were couple of years back :) I should have done better job with both code and documentation. Anyway I was utilizing your emulator code and felt that I should point out defects that I felt were present.

Fix for [sp+n] instruction length

I didn't have test case for this, but by reviewing the code it seemed that the if expressions for detecting [SP + next word] were missing. However, if that is already handled somewhere else, my fix is wrong.

In line 236 my fix is defect - it should be btype, just as you pointed out.

Fix for EX and operand b evaluation order
I think there is a defect in emulation if b operand is EX and instruction also sets EX as "overflow".

set EX, 3 shr EX, 1
Original implementation left EX holding the result of shift operation (0x0001) but DCPU spec 1.7 implies that "overflow" is set to EX after the b-operand is set. Thus correct result would be (0x8000)

Cleanup of switch structure

I have also unified the case statements. I changed them all to use the same format. They all use set(badd, x); return; instead of b=x; break;

Feature - configurable shift behaviour

Java specification states explicitly that "only the five lowest-order bits of the right-hand operand are used as the shift distance". In DCPU specification there is no such statement. My interpretation is that shift distance is then defined with 16 bits. However, the community de facto implementation seems to follow the Java standard. Correct implementation was not clear to me, so I made a switch SHIFT_DISTANCE_5_BITS to change the behaviour.

Fix for DIV instruction
Same commit also includes fix for division instruction. I cast b to long in EX calculation before left shift so that division is not polluted by sign extension.

@orlof orlof changed the title Emulator fixes for [sp+n] length and ex evaluation order Fix for [sp+n] instruction length, DIV EXtension, EX evaluation order and option for shift behaviour Apr 19, 2016
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