MIPS Assembler

[SteliosKaragiorgis]

This PR is to upload the MIPS Assembler. For now I updated the instruction.h

[SteliosKaragiorgis]

I dont know why it uploads the previous PRs

[mortbopet]

To avoid me repeating myself too much (😉) remember to go through all of the suggestions and comments that i placed on your previous PR, and also apply them here. The first part of any review process should be self review (e.g. you taking a critical look at the diff and thinking about whether a change is warranted.

Just as an example, a brief skim of the current diff tells me that there's MIPS constructs in the assembler, which is (just like we've discussed for VSRTL) a no go.

[SteliosKaragiorgis]

I made these changes because I used them on other file. Do i need to upload all the files or one by one?

[SteliosKaragiorgis]

So is it wrong that i used SignedMips ?

[SteliosKaragiorgis]

I uploaded mips relocations

[SteliosKaragiorgis]

Is it okay to upload the other PRs?

[mortbopet]

You should probably follow through with this PR before uploading the rest.

As the PR stands right now, i think it's a bit too small - you want small, atomic commits, yes; but ideally you also want something to use that PR. Right now, nothing is using this file so the things aren't even being built! Meaning that i (the reviewer) have no idea whether you're submitting code that actually compiles.

[SteliosKaragiorgis]

In this way i have to upload all the files of assembler because they are all used together

[SteliosKaragiorgis]

I hope it will be okay now

[mortbopet]

PR'ing the entire assembler at once seems like an OK granularity for the contribution; as a final thing, you should also include some assembler tests so we know that it's actually working.

Secondly, we just had a refactor of how instructions are represented in the assembler, s.t. they are no longer macro based but instead class based, so the PR needs a refactor to adhere to that (see 27fcc58).

[mortbopet]

... and maybe also watch out/wait for #314 to land, since that will require another change!

[SteliosKaragiorgis]

I never worked with tests, so I dont know how to implement a test for the assembler

[mortbopet]

I never worked with tests, so I dont know how to implement a test for the assembler

Then i would suggest you to look at the existing RISC-V tests (the ones i linked) which should give you a good idea about what assembler testing entails.

[SteliosKaragiorgis]

Okay then. For the assembler do I need to change it completely and do it like 27fcc58 ?

[SteliosKaragiorgis]

Do I need to do a new assembler test for MIPS or just edit this test ?

[mortbopet]

Do I need to do a new assembler test for MIPS or just edit this test ?

If you look at the folder hierarchy, you'll notice these tests are nested within risc-v-specific folders. As I've mentioned before, to have proper MIPS support in Ripes, you essentially have to do generally the same things as have been done for risc-v specific things - this also includes MIPS-specific tests.

Since we (at this point in the process) don't have any execution capability for MIPS-related stuff, i think it would suffice to add a test like the one you linked (https://github.com/mortbopet/Ripes/blob/master/test/tst_assembler.cpp) called tst_mips_assembler.cpp to ensure basic capabilities. Once execution capability is here, you should look into creating comprehensive self-tests like the assembly programs listed here: https://github.com/mortbopet/Ripes/tree/master/test/riscv-tests. Note that all of those self-tests have been grabbed from somewhere else - there's plenty of RISC-V ISA test suites out there, I'm sure there's also some for MIPS.

[SteliosKaragiorgis]

How can I connect the tst_mips_assembler.cpp with MIPS assembler?

[mortbopet]

@SteliosKaragiorgis as I've mentioned a few times before, the basic exercise here is to 1) look at what is done for RISC-V in Ripes and 2) do essentially the same thing for MIPS, but in a decoupled way such that the code is maintainable and of high quality. This implies that you have to spend time researching the codebase, and trying different things - I can't give exact instructions to every change.

For instance, a quick glance at tst_assembler should give you an idea about how you can call the MIPS assembler.