"srl (hl)" instruction not working correctly on emulator

[Hamberthm]

Hi! First of all, thanks for this AMAZING proyect. I owe to you my first steps into z80 ASM.

I've found and confirmed a bug on the srl (hl) instruction, it doesn't work as expected when executed on the emulator.

I've made an script that shifts sprites on memory. SRL (hl) should put 0 in the 8th bit, rotate right, and push the 1st bit to the carry flag, like so:

0 ---> rotate byte in (hl) ---> carry

In my code, I use this instruction tho shift the first bytes on the first block of each sprite. The rest of the blocks are handled by the instruction "rr (hl)" wich pushes the carry bit into the 8th bit before rotating. The last one seems to work OK, however SRL (hl) doesn't, as can be seen on the following screenshot from the emulator:

If I change the SRL of the first block to RR as the rest, then it works:

On real hardware, it works well if I use the SRL or RR instructions. So I can say it's confirmed and it's a problem of the emulator.

What I think the problem is; as seen on the screenshots, it seems that SRL (HL) fails to push the 1st bit to the carry flag, as the program fails to retrieve it and put in on the second character block (it becomes progressively empty as shown on the screenshot)

[gusmanb]

Just tested SRL (HL) and RR (HL) and they worked as expected.

Test this program:

Start:
    .model Spectrum48
    .org #8000

    ld hl, 50000
    ld (hl), 255
    srl (hl)
    rr (hl)

I recommend to open the register window, the watch expression window, add "+ [50000]" and debug step by step to see all the process.

First, the program loads 0xFF at address $C350. Then, it rotates right one bit, C becomes 1 and the byte at $C350 becomes 0x7F After that rotates through carry to the right, the byte becomes 0xBF and the carry remains 1

That's the expected behavior.

Can you create a minimal example showing the wrong behavior?

[Hamberthm]

Here, with a slight modification I can reproduce it. The thing is, it does it on the SECOND pass!! I added a second memory location also with FF loaded, and a jump to make it a loop. This is very basically how my script operates.

On the first pass, SRL operates on the first byte and pushes the 1st bit to carry as expected. Then the address count is incremented and RR uses the C flag to load it into the 8th bit of the second byte and rotates the rest of it. The problem appears at the start of the second pass. SRL operates again on the first byte and takes it from 7F (01111111) to 3F (00111111) as expected, but here clearly the carry flag is not set. It should be set as the 1st bit was 1 prior to rotation. Maybe SRL is taking the value of the 8th bit instead of the 1st?? Then it goes on and RR of course takes FF to 7F on the second byte, because the C flag was not set by SRL. (The second byte shouldn't change at all until the 9th pass of the script).

Start:
    .model Spectrum48
    .org #8000

    ld hl, 50000
    ld (hl), 255
    inc hl
    ld (hl), 255
    dec hl

redo:
    srl (hl)
    ex af,af' ;save C flag state
    inc hl
    ex af,af' ;retrieve it
    rr (hl)
    dec hl
    jp redo

The ex instructions are there to simulate my script. It saves the flag registers because in my script there's more code inbetween that can affect the C flag, here it can be omitted. Remember to watch the adress 50001 in addition to 50000.

Thanks!!

[gusmanb]

Confirmed, SRL is storing the left-most bit instead of the right-most one.

This causes the same problem:

Start:
    .model Spectrum48
    .org #8000

    ld hl, 50000
    ld (hl), 127
    srl (hl)

C becomes 0 instead of 1

Also, this only happens when SRL is executed on memory, using a register works as expected:

Start:
    .model Spectrum48
    .org #8000

    ld a, 127
    srl a

C becomes 1 as it should.

[gusmanb]

Bug found on Core/Spect.Net.SpectrumEmu/Cpu/Z80BitOperations.cs, line 2117:

_registers.F = s_RlCarry0Flags[srlVal];

Should be:

_registers.F = s_RrCarry0Flags[srlVal];

Will try to create a pull request this evening if I have time.

[Hamberthm]

Thanks so much! Any idea when will I be able to update to a fixed version? I'm using the Visual Studio extension. Sorry if it's a dumb question, this is all new to me.

Also, should I close this issue?

[gusmanb]

Sorry, no idea... @Dotneteer is the only one that can accept pull requests or release a new version...

Meanwhile, I have modified the faulty library using dnSpy and corrected the bug in it, I'm attaching the resulting library. Or if you prefer you can do the change yourself.

In any case, go to the VS extensions folder (%LocalAppData%\Microsoft\VisualStudio\\Extensions) and find the folder for spectnetide, then replace the DLL with the one I attached or edit it with dnSpy.

To edit with dnSpy open "Spect.Net.SpectrumEmu.dll" in it, go to "Spect.Net.SpectrumEmu.Cpu -> Z80Cpu ->SRL_HLi", edit the function, rename "Z80Cpu.s_RlCarry0Flags" to "Z80Cpu.s_RrCarry0Flags", save and generate the new dll.

Remember to do a backup of the original file in case you have any problem :)

About closing the issue, no, leave it open until @Dotneteer corrects the bug.

Cheers. Spect.Net.SpectrumEmu.zip

[Dotneteer]

Guys, because of familty reasons, I will have very little time for SpectNetIDE in the next three months. I have just merged back the PR that fixes this bug. Hopefully, in a few days, I'll have time to make a new release.

I'd be happy to add @gusmanb to the project as an admin so that he can do all the things (like merging PRs, releasing a new version, etc.) while I have limited time.. @gusmanb, what about this idea?

[gusmanb]

@Dotneteer I will try to create a compilation environment for the project, if I manage to get it working I would be more than glad to help creating patches and releases.

Will send you a mail (or telegram message) this weekend with the results :)

[Dotneteer]

@gusmanb, you're welcome! I've just sent you the invitation to collaborate :-)

[Hamberthm]

Thanks so so much @gusmanb for the fix! Up and running!

Also many thanks to @Dotneteer for the project. If it wasn't for this, I would never have gotten into z80 ASM. Please don't ever let it die. I just read a small book, and I'm here designing my own graphic engines. I've never went so far in any language! Thanks!!

PS: I know it's outside the topic, but the fact tha this wasn't discovered before makes me wonder, maybe SRL(HL) is not widely used? Is there a more efficient way of doing what I'm doing? I need to rotate bytes in memory, and I found that using directly SRL(HL) saves more cpu cycles than first loading the byte to a register and then doing the rotation, and then save it to memory again.

[Dotneteer]

@Hamberthm, you're welcome! It's my pleasure that you find this project helpful :-).

[gusmanb]

Solved in preview 10