> 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.
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. - Jason ----------------------------------------------------------- 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 [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
