> On Oct. 20, 2016, 2:35 p.m., Jason Lowe-Power wrote: > > Sorry for the slow reviewing. I have a few minor changes. > > > > First, when I apply the patch, I get a number of errors from the style > > checker. Make sure you're using the most up-to-date version of gem5 when > > you rebase your patch. > > > > Second, when I apply the patch and try to compile, I get a number of > > errors. Most are about the endianness functions: > > `build/RISCV/config/the_isa.hh:25:16: error: 'gtoh' is not a member of > > 'RiscvISA'` > > > > Third, I'm getting a number of "unused-variable" warnings (errors). If the > > variables are used for debugging (e.g., in asserts) you can use the macro > > M5_VAR_USED after the variable name and before the assignment. > > > > I'm not sure if the problem is that you've based this on an older version > > of gem5, or maybe we're just using different compilers (I'm use gcc-4.8). > > Alec Roelke wrote: > I ignored a few of the style warnings because I saw some other places in > gem5's code where the character limit was ignored. I'll go back through and > try to fix them all though. I think for remote_gdb there are some (a lot of) > warnings about "invalid control characters" that I wasn't sure how to fix. > > I built this using a system running Ubuntu 16.04, which has gcc 5.3 and > doesn't have any errors or warnings like you're describing. I tried > compiling it on a virtual machine running Ubuntu 14.04 and gcc 4.8.4, it also > works. I tested with gem5.debug and gem5.fast.
Thanks for fixing the style errors. Although there are still some places that don't follow the style guide in the code, we try to fix all of these as we find them. Definitely any new code should follow all of the style guidelines. I'm not sure why I was having errors yesterday. They've magically resolved themselves overnight. Hopefully you didn't spend too much time looking into it :). - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review8945 ----------------------------------------------------------- On Oct. 13, 2016, 4:48 p.m., Alec Roelke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3624/ > ----------------------------------------------------------- > > (Updated Oct. 13, 2016, 4:48 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11655:fce68047f694 > --------------------------- > 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.] > [Fixed spacing for some copyright attributions.] > [Replaced the rest of the file copies using hg copy.] > 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
