> 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!

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/.


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

Reply via email to