wokwi / rp2040js

A Raspberry Pi Pico Emulator in JavaScript
MIT License
371 stars 39 forks source link

Small suggestion about PC #1

Closed chegewara closed 3 years ago

chegewara commented 3 years ago

Hi, i really like how you are explaining and doing all this stuff, but i think you have 1 wrong assumption. PC should be increased before instruction is parsed/exacuted: https://github.com/wokwi/rp2040js/blob/master/src/rp2040.ts#L151

This will let you fix the branch tests:

    it('should execute a `b.n   .-18` instruction', () => {
      const rp2040 = new RP2040('');
      rp2040.PC = 9 * 2;
      rp2040.flash16[9] = 0xe7f6; // b.n    .-18
      rp2040.executeInstruction();
      expect(rp2040.PC).toEqual(0);
    });

    it('should execute a `bne.n .-6` instruction', () => {
      const rp2040 = new RP2040('');
      rp2040.PC = 9 * 2;
      rp2040.Z = false;
      rp2040.flash16[9] = 0xd1fc; // bne.n .-6
      rp2040.executeInstruction();
      expect(rp2040.PC).toEqual(12);
    });

For confirmation here is pseudocode for BL:

if ConditionPassed() then
 EncodingSpecificOperations();
 next_instr_addr = PC;
 LR = next_instr_addr<31:1> : ‘1’;
 BranchWritePC(PC + imm32);

As you can see next instruction address store is PC, which cant be PC to currently running instruction or this will stuck in recurent loop.

Another small suggestion about finding the right instruction in datasheet, you can use hex calculator. This snippet is good too to sign extend any value, like imm5, imm8, imm11:

function signExtend(value: number, bits: number) {
  return value & (1<<bits-1) ? 0x80000000 + (value & ((1<<bits-1)-1)) : value;
}

Thanks

urish commented 3 years ago

Thank you!

Regarding PC, the I found that the ARM® v6-M manual explains it in section A4.2.1 (Use of labels in UAL instruction syntax):

I will incorporate that into the code in our next session on Tuesday.

chegewara commented 3 years ago

Just as reminder (A.5.1.2):

Read the word-aligned PC value, that is, the address of the current instruction + 4, with bits [1:0] forced to
zero. This enables instructions such as ADR and LDR (literal) instructions to use PC-relative data addressing. 

Which works when you increment PC before execute instruction.

urish commented 3 years ago

I'm wondering why it uses "current instruction + 4" as the baseline, as instructions are only 2 bytes large (other than BL if I'm not wrong). It seems like even if we pre-increment, we'll have to add 2 to the value of PC when we are using it in offset-relative calculations

chegewara commented 3 years ago

It is exactly the same you posted earlier, just described with different words:

Thank you!

Regarding PC, the I found that the ARM® v6-M manual explains it in section A4.2.1 (Use of labels in UAL instruction syntax):

  • "The PC value of an instruction is its address plus 4 for a Thumb instruction"
  • "The Align(PC,4) value of an instruction is its PC value ANDed with 0xFFFFFFFC to force it to be word-aligned."

I will incorporate that into the code in our next session on Tuesday.

It is aligned PC only for few instructions, like ADR or LDR, but again, i played a bit with your code today and it works only when PC is incemented before instruction is excuted. Incementing PC at top is fixing also the problem with b.n or bne.n in tests. In code it works fine, because you actually are fooling code with https://github.com/wokwi/rp2040js/blob/master/src/rp2040.ts#L168

of course you dont need this line, which is moved into top: https://github.com/wokwi/rp2040js/blob/master/src/rp2040.ts#L284

sutaburosu commented 3 years ago

I'm wondering why it uses "current instruction + 4" as the baseline, as instructions are only 2 bytes large

Thumb instructions are only 2-bytes long, but after a BX opcode, on ARMs which support it, the ARM instruction set will be used instead; they are all 4-bytes long. This PC+4 offset goes back to the earliest revisions of ARM, before the Thumb instructions were added. I guess the choice was made to use PC+4 even in Thumb mode to reduce confusion.

urish commented 3 years ago

Interestingly, in non-Thumb mode it's actually PC+8: "The PC value of an instruction is its address plus 4 for a Thumb instruction, or plus 8 for an ARM instruction."

sutaburosu commented 3 years ago

Ah! On this ARM variant, PC actually points 2 instructions after the current one, because they deepened the pipeline from 3- to 5-stages. It surprises me that they chose to expose this extra 1 instruction offset. DEC/Intel's StrongARM also had a similar 5-stage pipeline, but still only a +1 instruction offset in PC.

urish commented 3 years ago

Cool, thanks for the link!

urish commented 3 years ago

Thanks again for the suggestion, @chegewara! I ended up implementing it at the beginning of last week's stream, and it actually helped fixed the bug we were looking at. So great timing too :-)

chegewara commented 3 years ago

No problem, and i saw last video. I will watch whole series, because its very nice and even if i dont plan to use it, most likely, it is very good to learn new things and new way of thinking. Good luck.