> On Oct. 12, 2016, 8:31 a.m., Andreas Hansson wrote: > > src/arch/riscv/process.cc, line 189 > > <http://reviews.gem5.org/r/3624/diff/8/?file=59265#file59265line189> > > > > Not that it really matters, but feel free to use "auto"
Actually I think I'll just use "int" as most people do. I'm not sure what I was thinking when I did this. > On Oct. 12, 2016, 8:31 a.m., Andreas Hansson wrote: > > src/arch/riscv/process.cc, line 52 > > <http://reviews.gem5.org/r/3624/diff/8/?file=59265#file59265line52> > > > > Is this based on a spec? If so, could you provide a pointer? I do remember referring to something when I was writing this, but it wasn't either of the volumes of the RISC-V specification and I can't find it anymore. I believe it was a source file somewhere in the riscv toolchain which had a comment describing the organization of program arguments upon entry. As for the values in the constructor of RiscvLiveProcess like stack_base, it didn't specify. I ended up using the ones from MIPS, except that the break point isn't rounded because that causes gem5 to crash when trying to run a program. > On Oct. 12, 2016, 8:31 a.m., Andreas Hansson wrote: > > ext/libelf/elf_common.h, line 175 > > <http://reviews.gem5.org/r/3624/diff/8/?file=59224#file59224line175> > > > > I think this particular line should not be changed and the line below > > should stick with the same spacing. That line is as it was before my patch. The other EM_X definitions all use a tab+spaces for alignment while EM_AARCH64 only uses spaces. I decided to align EM_RISCV with the others using a tab+spaces and leave EM_AARCH64 alone. Personally I think it should be changed so they all use only spaces, but I don't think this patch is the right place to do it. > On Oct. 12, 2016, 8:31 a.m., Andreas Hansson wrote: > > src/arch/riscv/microcode_rom.hh, line 2 > > <http://reviews.gem5.org/r/3624/diff/8/?file=59259#file59259line2> > > > > Was the spacing like this (in this and other files)? I think > > traditionally we did not align the legal entity names. It was (same for the others). I'll change it though. On Oct. 12, 2016, 8:31 a.m., Alec Roelke wrote: > > Some minor questions and cosmetic issues. > > > > Could you please also confirm that the files that are copied (and then > > modified) started out as hg copy (as previously discussed). > > > > Besides that I think this is looking really good. Thanks Alec! > > > > Steve, can we retire Alpha now to make up for the additional build and test > > time? :-) I went through and made sure to use hg copy on files that I copied. Ones that were similar but were from the original GitHub I didn't do this with, such as mmapped_ipr.hh (which I've learned is present in the other ISAs). Is there a way to check to make sure I did it right? Also, as a heads up, I'm adding a sixth patch in the next few days that will have a (more) proper implementation of the files in the linux/ directory. Currently the system calls technically work, but the structure of the data structures for calls like stat aren't what RISC-V programs are expecting. So far I've only run into this for stat and fstat. - Alec ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review8821 ----------------------------------------------------------- On Oct. 11, 2016, 5:17 p.m., Alec Roelke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3624/ > ----------------------------------------------------------- > > (Updated Oct. 11, 2016, 5:17 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11655:01b1d852e62e > --------------------------- > 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. > > Patch 2 will implement the multiply extension, RV64M; patch 3 will implement > the floating point (single- and double-precision) extensions, RV64FD; > patch 4 will implement the atomic memory instructions, RV64A, and patch 5 > will add support for timing, minor, and detailed CPU models that is missing > from the first four patches (such as handling locked memory). > > [Removed several unused parameters and imports from RiscvInterrupts.py, > RiscvISA.py, and RiscvSystem.py.] > [Fixed copyright information in RISC-V files copied from elsewhere that had > ARM licenses attached.] > [Reorganized instruction definitions in decoder.isa so that they are sorted > by opcode in preparation for the addition of ISA extensions M, A, F, D.] > [Fixed formatting of several files, removed some variables and > instructions that were missed when moving them to other patches, fixed > RISC-V Foundation copyright attribution, and fixed history of files > copied from other architectures using hg copy.] > [Fixed indentation of switch cases in isa.cc.] > [Reorganized syscall descriptions in linux/process.cc to remove large > number of repeated unimplemented system calls and added implmementations > to functions that have received them since it process.cc was first > created.] > Signed-off by: Alec Roelke > > > Diffs > ----- > > build_opts/RISCV PRE-CREATION > ext/libelf/elf_common.h 49cbf4bb0d36 > 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/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 > 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/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/locked_mem.hh PRE-CREATION > src/arch/riscv/microcode_rom.hh 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 49cbf4bb0d36 > src/base/loader/object_file.hh 49cbf4bb0d36 > src/cpu/BaseCPU.py 49cbf4bb0d36 > src/sim/process.cc 49cbf4bb0d36 > tests/test-progs/hello/bin/riscv/linux/hello 49cbf4bb0d36 > > 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
