> 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. > > Jason Lowe-Power wrote: > 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 :). > > Alec Roelke wrote: > Okay, I'll fix the rest of them, then (and in other patches too). > > I actually figured it out. In my original code, there was a weird bug in > isa/includes.isa where sim/system.hh had to be included out of order in, I > believe, the decoder or the errors you mentioned would pop up. I added an > include for it into registers.hh and it fixed it. > > Jason Lowe-Power wrote: > I see, I bet it was that I fixed some of the include orderings and got > that error. > > I have another question, though... > > I'm trying to test this out myself. I downloaded the compiler from > https://github.com/riscv/riscv-gnu-toolchain and then build hello with > `riscv64-unknown-linux-gnu-gcc ../../../src/hello.c -o hello -static > -march=RV64I`. > > When trying to run the binary I'm getting the error > ``` > panic: Unknown instruction 0x03755433 at pc 0x000000000002e1b8 > @ tick 318000 > ``` > > What am I doing wrong? I'm using configs/learning_gem5/part1/simple.py > with changes to use the atomic CPU instead of the timing. > > Thanks for all your work! > > Alec Roelke wrote: > I have been using riscv64-unknown-elf-gcc rather than > riscv64-unknown-gnu-linux-gcc. I don't know if riscv-gnu-toolchain will > build riscv64-unknown-elf-gcc, but you can get riscv-tools and build that, > and then you get riscv64-unknown-elf-gcc (and spike if you're interested in > playing around with that). I tried using riscv64-unknown-gnu-linux-gcc, but > I encountered a problem where gem5 would return from a function and then read > 0x00000000 as an instruction even though the instruction at the corresponding > PC in the assembly is valid. > > This doesn't appear to be your problem, though. Your instruction appears > to be a multiply instruction, which isn't part of RV64I. I'm not sure why > the compiler would do that when you specified -march=RV64I unless that's just > a suggestion and doesn't bind it to anything. However, I do recall that > riscv64-unknown-elf-gcc should not use a multiply when invoking printf with > only a single argument that's a string literal. > > I think your options are to either apply patch 2, which adds integer > multiply instructions, or try using riscv64-uknown-elf-gcc instead of > riscv64-unknown-gnu-linux-gcc. I would recommend the former, since trying > the latter might cause a different problem that I mentioned earlier. If all > you want to do is run something, there should also be a precompiled hello > binary in tests/test-progs/hello/bin/riscv/linux/.
I forgot to mention, I've just been using straight configs/example/se.py to run my code. I don't think that should make a difference, though. - Alec ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review8945 ----------------------------------------------------------- On Oct. 21, 2016, 6:12 p.m., Alec Roelke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3624/ > ----------------------------------------------------------- > > (Updated Oct. 21, 2016, 6:12 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11688:f84b3613acf4 > --------------------------- > 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.] > [Fixed style check errors and corrected unaligned memory accesses.] > Signed-off by: Alec Roelke > > > Diffs > ----- > > 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/system.hh PRE-CREATION > src/arch/riscv/stacktrace.hh PRE-CREATION > src/arch/riscv/process.cc PRE-CREATION > src/arch/riscv/pseudo_inst.hh PRE-CREATION > src/sim/process.cc b3d5f0e9e258 > src/base/loader/object_file.hh b3d5f0e9e258 > src/arch/riscv/utility.hh PRE-CREATION > src/arch/riscv/tlb.cc PRE-CREATION > src/arch/riscv/system.cc PRE-CREATION > src/arch/riscv/stacktrace.cc PRE-CREATION > src/arch/riscv/remote_gdb.hh PRE-CREATION > src/arch/riscv/remote_gdb.cc PRE-CREATION > src/arch/riscv/registers.hh PRE-CREATION > src/arch/riscv/process.hh PRE-CREATION > src/cpu/BaseCPU.py b3d5f0e9e258 > src/arch/riscv/vtophys.hh PRE-CREATION > src/base/loader/elf_object.cc b3d5f0e9e258 > src/arch/riscv/types.hh PRE-CREATION > src/arch/riscv/tlb.hh 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/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/RiscvSystem.py PRE-CREATION > src/arch/riscv/RiscvTLB.py PRE-CREATION > src/arch/riscv/SConscript PRE-CREATION > src/arch/riscv/SConsopts PRE-CREATION > build_opts/RISCV PRE-CREATION > ext/libelf/elf_common.h b3d5f0e9e258 > src/arch/riscv/RiscvISA.py PRE-CREATION > src/arch/riscv/RiscvInterrupts.py 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
