> On Sept. 15, 2016, 2:50 p.m., Jason Lowe-Power wrote: > > Thanks for the contribution. It looks pretty good! Could you give us a > > preview as to the other 4 patches? What is missing in this one? > > > > I know nothing of how the ISA stuff works in gem5, but I reviewed most of > > the other parts of this patch. > > > > Some high-level notes: > > > > a.out probably shouldn't be in the patch. > > > > Make sure you've updated all of the copyright information. > > > > Have you run any of the RISC-V test suite on your code? > > (https://riscv.org/software-tools/riscv-tests/) In fact, we could probably > > incorporate some (all?) of that into the regression tests. How much we > > include depends on how long the tests take. > > Alec Roelke wrote: > Yeah, I just noticed a.out. Is there a way to prevent SCons from > producing it? > > Which copyright information are you referring to? I went through and > added it where I thought it was necessary, but I'm not very familiar with > copyrights and licensing so I could have gotten it wrong. > > I have tried running the some of the RISC-V tests, but they don't seem > have the structure that GEM5 expects--for example, the entry point of > rv64ui-p-add as defined by the ELF file doesn't exist in the disassembly. > > Jason Lowe-Power wrote: > I don't know how to get rid of a.out. If you find out, let me know too! > > For the copyright. It seemed there were a number of files that you > created that had other people's name on them (e.g., > src/arch/riscv/RiscvInterrupts.py). Sincd you changed pretty much every line > in that file, and created the file, you can add your copyright and author > name. > > Finally, that's unfortunate about the RISC-V tests. Is there a way to > slightly modify the loading of gem5 to support them? Maybe this is something > for a separate patch. It would be really great to run tests like this.
The files that I didn't put my name in were not created by me; this project originated from earlier work we found on GitHub (basically from searching Google to see if someone had already done it). While there are heavy modifications that I made in general as beforehand very little was implemented, the files that I didn't touch at all, or that I only changed formatting for, I left with the original copyright information. Same for files I copied from MIPS without editing (I think that's only the TLB though). The RISC-V GNU toolchain comes with an architecture simulator that is basically a dressed-down version of gem5's atomic CPU model and is able to run those tests, so it could be possible to modify gem5's ELF loader to work. On the other hand, if you want to run code compiled with the GNU toolchain on their simulator, you have to also use their proxy kernel, which crashes when you try to give it a RISC-V ISA test to run. I think the simulator probably uses a different entry point than the ELF file defines and the proxy kernel is responsible for reading it to determine the proper entry point, which gem5 does internally. Not to mention that the tests don't actually have any output themselves--whatever hardware or software is performing them is responsible for knowing if they've passed or not. - Alec ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review8722 ----------------------------------------------------------- On Sept. 15, 2016, 3:19 p.m., Alec Roelke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3624/ > ----------------------------------------------------------- > > (Updated Sept. 15, 2016, 3:19 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11620:849620cf01a3 > --------------------------- > arch: [Patch 1/5] Added RISC-V base instruction set RV64I > > First of five patches adding RISC-V to GEM5. This patch introduces the > base 64-bit ISA (RV64I) in src/arch/riscv for use with syscall emulation. > The multiply, floating point, and atomic memory instructions will be added > in additional patches, as well as support for more detailed CPU models. > The loader is also modified to be able to parse RISC-V ELF files, and a > "Hello world!" example for RISC-V is added to test-progs. > > > Diffs > ----- > > src/arch/riscv/isa/base.isa PRE-CREATION > src/arch/riscv/isa/bitfields.isa PRE-CREATION > src/arch/riscv/isa/decoder.isa PRE-CREATION > src/arch/riscv/isa/formats/basic.isa PRE-CREATION > src/arch/riscv/isa/formats/formats.isa PRE-CREATION > src/arch/riscv/isa/formats/mem.isa PRE-CREATION > src/arch/riscv/isa/formats/type.isa PRE-CREATION > src/arch/riscv/isa/formats/unknown.isa PRE-CREATION > src/arch/riscv/isa/includes.isa PRE-CREATION > src/arch/riscv/faults.hh PRE-CREATION > src/arch/riscv/faults.cc PRE-CREATION > src/arch/riscv/idle_event.hh PRE-CREATION > src/arch/riscv/idle_event.cc PRE-CREATION > src/arch/riscv/interrupts.hh PRE-CREATION > src/arch/riscv/interrupts.cc PRE-CREATION > src/arch/riscv/isa.hh PRE-CREATION > src/arch/riscv/isa.cc PRE-CREATION > build_opts/RISCV PRE-CREATION > ext/libelf/elf_common.h 8bc53d5565ba > src/arch/riscv/RiscvISA.py PRE-CREATION > src/arch/riscv/RiscvInterrupts.py PRE-CREATION > src/arch/riscv/RiscvSystem.py PRE-CREATION > src/arch/riscv/RiscvTLB.py PRE-CREATION > src/arch/riscv/SConscript PRE-CREATION > src/arch/riscv/SConsopts PRE-CREATION > src/arch/riscv/decoder.hh PRE-CREATION > src/arch/riscv/decoder.cc PRE-CREATION > src/arch/riscv/mmapped_ipr.hh PRE-CREATION > src/arch/riscv/pagetable.hh PRE-CREATION > src/arch/riscv/pagetable.cc PRE-CREATION > src/arch/riscv/pra_constants.hh PRE-CREATION > src/arch/riscv/process.hh PRE-CREATION > src/arch/riscv/process.cc PRE-CREATION > src/arch/riscv/pseudo_inst.hh PRE-CREATION > src/arch/riscv/registers.hh PRE-CREATION > src/arch/riscv/remote_gdb.hh PRE-CREATION > src/arch/riscv/remote_gdb.cc PRE-CREATION > src/arch/riscv/stacktrace.hh PRE-CREATION > src/arch/riscv/stacktrace.cc PRE-CREATION > src/arch/riscv/system.hh PRE-CREATION > src/arch/riscv/system.cc PRE-CREATION > src/arch/riscv/tlb.hh PRE-CREATION > src/arch/riscv/tlb.cc PRE-CREATION > src/arch/riscv/types.hh PRE-CREATION > src/arch/riscv/utility.hh PRE-CREATION > src/arch/riscv/vtophys.hh PRE-CREATION > src/base/loader/elf_object.cc 8bc53d5565ba > src/base/loader/object_file.hh 8bc53d5565ba > src/cpu/BaseCPU.py 8bc53d5565ba > src/sim/process.cc 8bc53d5565ba > tests/test-progs/hello/bin/riscv/linux/hello 8bc53d5565ba > src/arch/riscv/isa/main.isa PRE-CREATION > src/arch/riscv/isa/operands.isa PRE-CREATION > src/arch/riscv/isa_traits.hh PRE-CREATION > src/arch/riscv/kernel_stats.hh PRE-CREATION > src/arch/riscv/linux/linux.hh PRE-CREATION > src/arch/riscv/linux/linux.cc PRE-CREATION > src/arch/riscv/linux/process.hh PRE-CREATION > src/arch/riscv/linux/process.cc PRE-CREATION > src/arch/riscv/linux/system.hh PRE-CREATION > src/arch/riscv/linux/system.cc PRE-CREATION > src/arch/riscv/locked_mem.hh PRE-CREATION > src/arch/riscv/microcode_rom.hh PRE-CREATION > > Diff: http://reviews.gem5.org/r/3624/diff/ > > > Testing > ------- > > > Thanks, > > Alec Roelke > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
