ucb-bar / esp-llvm

UCB-BAR fork of LLVM! NOT UPSTREAM RISCV LLVM
Other
123 stars 55 forks source link

Fix encoding of all assembly instructions that can currently be parsed #9

Closed jordypotman closed 9 years ago

jordypotman commented 9 years ago

This fixes the encoding of all assembly instructions that can currently be parsed. The encoding that is tested for in the CHECK lines of the tests has been verified by comparing it to the encoding generated by the assembler from the GNU toolchain.

colinschmidt commented 9 years ago

Other than that one change things look good.

jordypotman commented 9 years ago

Thanks for the review! I have pushed another commit which puts RS1 and RS2 in the correct bit positions. The src1 and src2 operands in the input operand list were swapped which explains why the encoding was still correct.

Some of the other instruction formats (which I did not change in this pull request) look like they may have the same issue, but we should run into that when we add encoding checks for instructions that use those formats.

colinschmidt commented 9 years ago

It turns out this caused some test cases in my personal verification script to fail after the switch but I was able to figure it out quickly so I just pull it and added my fix up afterwards (swapping slt[u] operands).

At some point in the past I was looking into getting the llvm test-suite (http://llvm.org/docs/TestingGuide.html#test-suite) running on spike (which is what my verification script does in a very limited fashion). If we had this running it would be easier for all of us to see what is failing currently and what is broken as we make changes.

Thanks for the help!

jleidel commented 9 years ago

Colin, has there ever been any thought to putting together a nightly build+test system? One of the items on my todo list is to build a working kernel image for spike such that I can do some extensive testing with the openmp port to riscv.

--John D. Leidel

On Jan 27, 2015, at 7:17 PM, Colin Schmidt notifications@github.com wrote:

It turns out this caused some test cases in my personal verification script to fail after the switch but I was able to figure it out quickly so I just pull it and added my fix up afterwards (swapping slt[u] operands).

At some point in the past I was looking into getting the llvm test-suite (http://llvm.org/docs/TestingGuide.html#test-suite) running on spike (which is what my verification script does in a very limited fashion). If we had this running it would be easier for all of us to see what is failing currently and what is broken as we make changes.

Thanks for the help!

— Reply to this email directly or view it on GitHub.

jordypotman commented 9 years ago

When I was rebasing riscv-llvm to LLVM trunk I also experimented with compiling the LLVM test-suite for RISC-V and running it on spike to make sure that I was not introducing any regressions. For this I added support to the LLVM test-suite tree for configuring it for RISC-V. With a lot of make options and some scripts I was at the time able to get 140 of the 369 C tests in it to compile and a subset of those 140 also ran correctly.

If it is useful I can push the modified LLVM test-suite tree to GitHub and add a README that describes how I build and run it. I think I just managed to get it working again with a riscv-gnu-toolchain based riscv-as and riscv-gcc (only used for linking).

Running the LLVM and clang regression tests (using make check-all) might also give us some additional test coverage although we do not have a lot of RISC-V specific tests in there yet. For that we might want to move the parts of the valid-* RISC-V MC tests that are currently still failing to valid-xfail-* files. Currently I am usually running the tests with the failing parts commented out to see if the rest of the test still passes.

I saw that the riscv-tools repository started to use https://travis-ci.org. Maybe we can use that for building and testing riscv-llvm as well?

colinschmidt commented 9 years ago

We actually have a jenkins bot(not public) setup but it currently only checks that the code compiles. I just updated to run make check as well since we have more interesting tests there now.

The llvm test-suite seems pretty extensive so if that were running on a nightly/weekly basis we could see how the backend was really doing.

On Tue, Jan 27, 2015 at 6:22 PM, jleidel notifications@github.com wrote:

Colin, has there ever been any thought to putting together a nightly build+test system? One of the items on my todo list is to build a working kernel image for spike such that I can do some extensive testing with the openmp port to riscv.

--John D. Leidel

On Jan 27, 2015, at 7:17 PM, Colin Schmidt notifications@github.com wrote:

It turns out this caused some test cases in my personal verification script to fail after the switch but I was able to figure it out quickly so I just pull it and added my fix up afterwards (swapping slt[u] operands).

At some point in the past I was looking into getting the llvm test-suite (http://llvm.org/docs/TestingGuide.html#test-suite) running on spike (which is what my verification script does in a very limited fashion). If we had this running it would be easier for all of us to see what is failing currently and what is broken as we make changes.

Thanks for the help!

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/ucb-bar/riscv-llvm/pull/9#issuecomment-71770015.

colinschmidt commented 9 years ago

Hmm I thought there would be more C tests in the test-suite. Right now I have a set of scripts that runs a small subset of the deja-gnu test suite i.e. the single c file tests. I made all the tests abort when they fail so its simpler to check their success/failure.

If you have a modified llvm test-suite tree it would be cool to look at and maybe use it.

I think running make check-all is a good idea too and I like moving the currently failing tests to xfail until we get around to actually fixing them.

I can look into travis. I believe the only reason we have jenkins is because it works better for non-public projects.

On Wed, Jan 28, 2015 at 12:04 PM, Colin Schmidt colins@eecs.berkeley.edu wrote:

We actually have a jenkins bot(not public) setup but it currently only checks that the code compiles. I just updated to run make check as well since we have more interesting tests there now.

The llvm test-suite seems pretty extensive so if that were running on a nightly/weekly basis we could see how the backend was really doing.

On Tue, Jan 27, 2015 at 6:22 PM, jleidel notifications@github.com wrote:

Colin, has there ever been any thought to putting together a nightly build+test system? One of the items on my todo list is to build a working kernel image for spike such that I can do some extensive testing with the openmp port to riscv.

--John D. Leidel

On Jan 27, 2015, at 7:17 PM, Colin Schmidt notifications@github.com wrote:

It turns out this caused some test cases in my personal verification script to fail after the switch but I was able to figure it out quickly so I just pull it and added my fix up afterwards (swapping slt[u] operands).

At some point in the past I was looking into getting the llvm test-suite (http://llvm.org/docs/TestingGuide.html#test-suite) running on spike (which is what my verification script does in a very limited fashion). If we had this running it would be easier for all of us to see what is failing currently and what is broken as we make changes.

Thanks for the help!

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/ucb-bar/riscv-llvm/pull/9#issuecomment-71770015.

jordypotman commented 9 years ago

Ok, when I have the modified llvm test-suite up on GitHub I'll send a message to the sw-dev mailing list so anyone else that might be interested is aware of it and we can stop abusing this pull request for this discussion :-)