Skip to content

Add MIPS32 accumulating instructions #282

Closed
3 tasks done
pavelkryukov opened this issue Feb 10, 2018 · 12 comments
Closed
3 tasks done

Add MIPS32 accumulating instructions #282

pavelkryukov opened this issue Feb 10, 2018 · 12 comments
Assignees
Labels
2 Small features, tests coverage, simple laboratory works enhancement Adds a new feature to simulation. S1 — ISA To solve the issue, you need knowledge about MIPS or RISC-V ISA

Comments

@pavelkryukov
Copy link
Member

pavelkryukov commented Feb 10, 2018

MIPS32 introduces 4 accumulation instructions: madd, maddu, msub, msubu. Unlike common instructions, they don't overwrite the contents of HI/LO register pair, but accumulate results:

    hilo += rs * rt;

The roadmap for implementation:

  • Add accumulation feature to register file
  • Introduce instruction to MIPS decoding flow
  • Add a flag to perform accumulation instead of writing
@pavelkryukov pavelkryukov added enhancement Adds a new feature to simulation. 2 Small features, tests coverage, simple laboratory works S1 — ISA To solve the issue, you need knowledge about MIPS or RISC-V ISA labels Feb 10, 2018
@pavelkryukov pavelkryukov added this to the Enhanced MIPS milestone Feb 10, 2018
@pavelkryukov
Copy link
Member Author

@AndrewSultan

The idea of this instructions is to have an additional ALU which operates over HI/LO registers. To model it, you have to introduce new interfaces to RF class which does addition or subtraction instead of simple overwrite.

Additionally, such instructions cannot deliver data to bypass logic. Please consult with @denislos to understand the best way to change model properly.

This was referenced Mar 26, 2018
@AndreiZoltan
Copy link
Contributor

For an unknown reason the madd test failed, but maddu test passed.

@pavelkryukov
Copy link
Member Author

uint64 hi_lo = static_cast<uint64>(entry_hi.value);

That looks suspicious, as all operations are unsigned. However I don't see an obvious bug...

What are the operands for failing test, actual output, and expected output?

@AndreiZoltan
Copy link
Contributor

0x400b54: madd $a0, $a0 [ $hi = 0x1, $lo = 0x0 ]
0x400b58: mfhi $v1 [ $v1 = 0x2 ]


Actual msg:
[ DEATH ] Mismatch:
[ DEATH ] Checker output: 0x400b58: mfhi $v1 [ $v1 = 0x2 ]
[ DEATH ] PerfSim output: 0x400b58: mfhi $v1 [ $v1 = 0x1 ]
[ DEATH ]
[ FAILED ] Perf_Sim.Run_Full_Trace (14 ms)
[ RUN ] Perf_Sim.Run_SMC_Trace
[ OK ] Perf_Sim.Run_SMC_Trace (5 ms)
[----------] 2 tests from Perf_Sim (19 ms total)

@pavelkryukov
Copy link
Member Author

Double-check that condition:

auto is_bypassible() const { return !this->is_conditional_move() || this->is_accumulating_instr(); }

@pavelkryukov
Copy link
Member Author

Denis @denislos, could you please take a look? It seems that MADD bypasses 0x1 to HI register, while it should be prevented by is_bypassible() check.

@pavelkryukov
Copy link
Member Author

void execute_madd() { v_dst = mips_multiplication<int32>(v_src1, v_src2); }

BTW, you don't need these functions as they do the same thing as execute_mult and execute_mult; all special behavior is controlled by operation type

@denislos
Copy link
Contributor

denislos commented Mar 27, 2018

There is a strange thing, I have run the tests and got this


fetch cycle 2491: 0x400b3c: 0x0x400b3c: mfhi $v1
decode cycle 2491: 0x400b38: madd $a0, $a0
execute cycle 2491: 0x400b34: lui $a0, 0x1 [ $a0 = 0x10000 ]
memory cycle 2491: 0x400b30: mthi $zero [ $hi = 0x0 ]
wb cycle 2492: 0x400b30: mthi $zero [ $hi = 0x0 ]
Executed instructions: 944

decode cycle 2492: 0x400b3c: mfhi $v1 (data hazard)
execute cycle 2492: 0x400b38: madd $a0, $a0 [ $hi = 0x1, $lo = 0x0 ]
memory cycle 2492: 0x400b34: lui $a0, 0x1 [ $a0 = 0x10000 ]
wb cycle 2493: 0x400b34: lui $a0, 0x1 [ $a0 = 0x10000 ]
Executed instructions: 945

decode cycle 2493: 0x400b3c: mfhi $v1 (data hazard)
execute cycle 2493: bubble
memory cycle 2493: 0x400b38: madd $a0, $a0 [ $hi = 0x1, $lo = 0x0 ]
In DECODE_2_FETCH_STALL port data was added at 2492 clock and will not be readed`


It looks like that the fetch stage does not handle stalls in case of a cache miss. I have changed fetch a little bit in order to fix it
template
void Fetch::ignore( Cycle cycle)
{
/* ignore PC from other ports in the case of cache miss */
rp_external_target->ignore( cycle);
rp_hold_pc->ignore( cycle);
rp_target->ignore( cycle);
rp_stall->ignore( cycle);
rp_hit_or_miss->ignore( cycle);
}

Now it passes all the tests.

@pavelkryukov
Copy link
Member Author

pavelkryukov commented Mar 27, 2018

@denislos Thank you for your prompt response.
There is no problem with bypass, right?

@pavelkryukov
Copy link
Member Author

In DECODE_2_FETCH_STALL port data was added at 2492 clock and will not be readed`

@AndrewSultan Do you have the same message as Denis has?

@denislos
Copy link
Contributor

Thank you. There is no problem with bypass, right?

It seems so.

@pavelkryukov
Copy link
Member Author

Please proceed by updating submodule and implementing subtracting instructions. I think the best strategy is to return int8 for is_accumulating(): -1 does subtraction, 1 does addition, 0 does nothing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
2 Small features, tests coverage, simple laboratory works enhancement Adds a new feature to simulation. S1 — ISA To solve the issue, you need knowledge about MIPS or RISC-V ISA
Projects
None yet
Development

No branches or pull requests

3 participants