Skip to content

Implement MIPS III doubleword memory instructions #215

Closed
pavelkryukov opened this issue Nov 9, 2017 · 17 comments
Closed

Implement MIPS III doubleword memory instructions #215

pavelkryukov opened this issue Nov 9, 2017 · 17 comments
Assignees
Labels
0 This task has the owner who does not participate in scoring system.

Comments

@pavelkryukov
Copy link
Member

pavelkryukov commented Nov 9, 2017

MIPS III introduced a lot of doubleword instructions which operate with 64 bit extended registers. They are rarely used as most of MIPS HW is 32-bit, but we might be interested to keep them as well

  • ld
  • ldl
  • ldr
  • lwu
  • sd
  • sdl
  • sdr
@pavelkryukov pavelkryukov added 3 Features of medium complexity or infrastructure enhancements enhancement Adds a new feature to simulation. S1 — ISA To solve the issue, you need knowledge about MIPS or RISC-V ISA labels Nov 9, 2017
@pavelkryukov pavelkryukov added this to the 64-bit MIPS milestone Nov 9, 2017
@pavelkryukov pavelkryukov changed the title mplement MIPS III doubleword memory instructions Implement MIPS III doubleword memory instructions Nov 9, 2017
@inedostoev inedostoev self-assigned this Dec 13, 2017
@pavelkryukov
Copy link
Member Author

@inedostoev Any progress?

@inedostoev
Copy link
Contributor

I and @TimofeevAlex would like to do it in the coming days.

@pavelkryukov
Copy link
Member Author

@inedostoev Igor reported you faced some issue, could you please explain?

@inedostoev
Copy link
Contributor

Yes, I try. As I understend, in MIPS III introduce doubleword instructions, and I thought about it realization. On wikipedia I read, for example realization of instruction ld for 32-bit register, I should download from memory this doubleword, and write this word in 2 registers, first 4 byte in first register and next 4 byte in next register. But this is strange. So, now I have a question. Should I do that I describe before, or I should change in class RF all 32-bit register to 64-bit?

@pavelkryukov
Copy link
Member Author

Should I do that I describe before, or I should change in class RF all 32-bit register to 64-bit?

I think the only way is to have 64-bit registers. I suggest to have a size field to RF operations, so 32-bit operations will update only low parts, 64-bit parts update 64 bits, and division/multiplication updates two 64-bit registers. As a result, get_dst_value will return 128 bits — please use boost types I placed to infra directory.

@inedostoev
Copy link
Contributor

I change variable v_dst, now I have uint128 v_dst, but what NO_VAL i should use for the first init?

@pavelkryukov
Copy link
Member Author

pavelkryukov commented Mar 5, 2018

Please invent one :-) Try to use value different to other NO_VALs.

@inedostoev
Copy link
Contributor

When I change v_dst from 64-bit to 128-bit, I got new problems. This problem with perf_sim. This is a log

core/perf_sim.cpp:364:16:   required from here
core/perf_sim.cpp:296:5: error: no matching function for call to ‘WritePort<long unsigned int>::write(uint128, Cycle&)’
     wp_execute_2_execute_bypass->write( instr.get_v_dst(), cycle);
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from core/perf_sim.h:15:0,
                 from core/perf_sim.cpp:6:
./infra/ports/ports.h:246:24: note: candidate: void WritePort<T>::write(const T&, Cycle) [with T = long unsigned int]
 template<class T> void WritePort<T>::write( const T& what, Cycle cycle)
                        ^~~~~~~~~~~~
./infra/ports/ports.h:246:24: note:   no known conversion for argument 1 from ‘uint128 {aka boost::multiprecision::number<boost::multiprecision::backends::cpp_int_backend<128, 128, (boost::multiprecision::cpp_integer_type)0, (boost::multiprecision::cpp_int_check_type)0, void> >}’ to ‘const long unsigned int&’
core/perf_sim.cpp: In instantiation of ‘void PerfSim<ISA>::clock_memory(Cycle) [with ISA = MIPS]’:

As I understang, it is problem with converting 128-bit to 64-bit, because get_v_dst() now return 128-bit . But should all variables be 64-bit in this module? Or I should change types to uint128 too? (change for wp_execute_2_execute_bypass , etc.)

@pavelkryukov
Copy link
Member Author

Yep, 128-bit should poison everything. You may consult @denislos for data bypassing unit internals.

@inedostoev
Copy link
Contributor

Now I have problems with unit test, with func_sim.

func_sim/t/unit_test.cpp:41: Failure
Death test: {{ mips.run_no_limit( valid_elf_file); } ::exit(0); }
    Result: died but not with expected exit code:
            Terminated by signal 6 (core dumped)
Actual msg:
[  DEATH   ] test.out: infra/memory/memory.cpp:93: void FuncMemory::write(uint64, Addr, uint32): Assertion `addr != 0' failed.
[  DEATH   ] 

I understand, that in function write() some instr put NULL address, but I cannot understand,what is instr it could be.
Can you tell me in which direction to think?
I can attach pull request.

@pavelkryukov
Copy link
Member Author

Run the trace in functional simulator in verbose mode (./mipt-mips -b /path/to/your/trace -f -d)

@inedostoev
Copy link
Contributor

Ok, thank you, I will try

@inedostoev
Copy link
Contributor

Good day. I finally figured out with FuncSim test. Now I passed it. But I failed another test, namely PerfSim.The log below.

[==========] Running 5 tests from 2 test cases.
[----------] Global test environment set-up.
[----------] 3 tests from Perf_Sim_init
[ RUN      ] Perf_Sim_init.Process_Correct_Args_Of_Constr
/home/inedostoev/mipt-mips/simulator/core/t/unit_test.cpp:21: Failure
Death test: {{ PerfSim<MIPS> mips( false); } ::exit(0); }
    Result: died but not with expected exit code:
            Exited with exit status 1
Actual msg:
[  DEATH   ] No ReadPorts for MEMORY_2_EXECUTE_BYPASS key
[  DEATH   ] 
[  FAILED  ] Perf_Sim_init.Process_Correct_Args_Of_Constr (3 ms)
[ RUN      ] Perf_Sim_init.Make_A_Step
/home/inedostoev/mipt-mips/simulator/core/t/unit_test.cpp:27: Failure
Death test: {{ PerfSim<MIPS>( false).run( valid_elf_file, 1); } ::exit(0); }
    Result: died but not with expected exit code:
            Exited with exit status 1
Actual msg:
[  DEATH   ] No ReadPorts for MEMORY_2_EXECUTE_BYPASS key
[  DEATH   ] 
[  FAILED  ] Perf_Sim_init.Make_A_Step (3 ms)
[ RUN      ] Perf_Sim_init.Process_Wrong_Args
/home/inedostoev/mipt-mips/simulator/core/t/unit_test.cpp:34: Failure
Death test: PerfSim<MIPS>( false).run( "./1234567890/qwertyuop", 1)
    Result: died but not with expected error.
  Expected: ERROR.*
Actual msg:
[  DEATH   ] No ReadPorts for MEMORY_2_EXECUTE_BYPASS key
[  DEATH   ] 
[  FAILED  ] Perf_Sim_init.Process_Wrong_Args (5 ms)
[----------] 3 tests from Perf_Sim_init (11 ms total)

[----------] 2 tests from Perf_Sim
[ RUN      ] Perf_Sim.Run_Full_Trace

Problem with ports, but I change all to uint128. What could be the problem?

@pavelkryukov
Copy link
Member Author

No ReadPorts for MEMORY_2_EXECUTE_BYPASS key

Double-check type of wp_memory_2_execute_bypass — the write port that uses the key.

@pavelkryukov
Copy link
Member Author

pavelkryukov commented Mar 13, 2018

Sorry, my bad

rps_stages_2_execute_sources_bypass — this read port should be checked

@pavelkryukov
Copy link
Member Author

pavelkryukov commented Mar 13, 2018

Do you guys have any tests for behavior of these instructions? I just realized that our beloved SPIM's tt.core.s is 32-bit only.

@inedostoev
Copy link
Contributor

Of course, we add simple tests

@pavelkryukov pavelkryukov added 0 This task has the owner who does not participate in scoring system. and removed 3 Features of medium complexity or infrastructure enhancements enhancement Adds a new feature to simulation. S1 — ISA To solve the issue, you need knowledge about MIPS or RISC-V ISA labels May 6, 2018
@pavelkryukov pavelkryukov self-assigned this May 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
0 This task has the owner who does not participate in scoring system.
Projects
None yet
Development

No branches or pull requests

3 participants