Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-11-01 Thread Tony Gutierrez
> On Oct. 31, 2016, 4:33 p.m., Tony Gutierrez wrote: > > src/arch/riscv/faults.hh, line 51 > > > > > > Are these duplicate values right? > > Alec Roelke wrote: > Yes. RISC-V doesn't distinguish between misaligned

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-11-01 Thread Tony Gutierrez
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review9015 --- Ship it! Ship It! - Tony Gutierrez On Oct. 31, 2016, 6:29 p.m., Alec

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-31 Thread Alec Roelke
> On Oct. 31, 2016, 11:33 p.m., Tony Gutierrez wrote: > > src/arch/riscv/faults.hh, line 51 > > > > > > Are these duplicate values right? Yes. RISC-V doesn't distinguish between misaligned addresses with store

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-31 Thread Alec Roelke
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/ --- (Updated Nov. 1, 2016, 1:29 a.m.) Review request for Default. Repository: gem5

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-31 Thread Tony Gutierrez
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review9006 --- A few minor things, otherwise it looks pretty good.

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-27 Thread Jason Lowe-Power
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review8997 --- Ship it! Sorry for the delay. I finally got everything working and it

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-21 Thread Jason Lowe-Power
> 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-21 Thread Alec Roelke
> 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-21 Thread Jason Lowe-Power
> 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-21 Thread Alec Roelke
> 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-21 Thread Alec Roelke
> 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-21 Thread Jason Lowe-Power
> 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-21 Thread Alec Roelke
> On Oct. 20, 2016, 7:10 p.m., Steve Reinhardt wrote: > > src/arch/riscv/microcode_rom.hh, line 38 > > > > > > Do you really need a microcode ROM? Seems like this whole file should > > not be necessary. I don't think it

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-21 Thread Alec Roelke
--- 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-21 Thread Alec Roelke
> 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-21 Thread Jason Lowe-Power
> 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-20 Thread Alec Roelke
> 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-20 Thread Steve Reinhardt
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review8952 --- Looks good overall! Just a few minor nits, really.

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-20 Thread Jason Lowe-Power
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review8945 --- Sorry for the slow reviewing. I have a few minor changes. First, when I

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-13 Thread Andreas Hansson
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review8836 --- Ship it! Looks good to me. It would be good if someone else could

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-13 Thread Andreas Hansson
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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-13 Thread Alec Roelke
--- 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-13 Thread Alec Roelke
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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-13 Thread Andreas Hansson
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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-12 Thread Alec Roelke
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/ --- (Updated Oct. 12, 2016, 4:46 p.m.) Review request for Default. Repository: gem5

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-12 Thread Alec Roelke
> On Oct. 12, 2016, 8:31 a.m., Andreas Hansson wrote: > > src/arch/riscv/process.cc, line 189 > > > > > > 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-12 Thread Andreas Hansson
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review8821 --- ext/libelf/elf_common.h (line 175)

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-11 Thread Alec Roelke
--- 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-10-10 Thread Alec Roelke
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/ --- (Updated Oct. 10, 2016, 4:59 p.m.) Review request for Default. Repository: gem5

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-29 Thread Alec Roelke
> On Sept. 27, 2016, 8:53 p.m., Andreas Hansson wrote: > > Thanks again for getting this in shape. There are a few minor questions and > > comment on the patch, the most important one is related to the copyrights. > > > > Also, I assume you have used hg copy for the files that are more or less

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-29 Thread Alec Roelke
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/ --- (Updated Sept. 29, 2016, 6:57 p.m.) Review request for Default. Repository: gem5

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-28 Thread Alec Roelke
> On Sept. 27, 2016, 8:53 p.m., Andreas Hansson wrote: > > Thanks again for getting this in shape. There are a few minor questions and > > comment on the patch, the most important one is related to the copyrights. > > > > Also, I assume you have used hg copy for the files that are more or less

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-27 Thread Alec Roelke
> On Sept. 27, 2016, 8:53 p.m., Andreas Hansson wrote: > > Thanks again for getting this in shape. There are a few minor questions and > > comment on the patch, the most important one is related to the copyrights. > > > > Also, I assume you have used hg copy for the files that are more or less

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-27 Thread Andreas Hansson
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review8737 --- Thanks again for getting this in shape. There are a few minor questions

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-16 Thread Alec Roelke
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/ --- (Updated Sept. 16, 2016, 4:49 p.m.) Review request for Default. Repository: gem5

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-16 Thread Alec Roelke
> On Sept. 15, 2016, 8:26 p.m., Andreas Hansson wrote: > > src/arch/riscv/RiscvISA.py, line 3 > > > > > > Regarding the license edits, the "normal" 3-clause BSD is the bottom > > two paragraphs. > > > > I would

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-15 Thread Alec Roelke
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/ --- (Updated Sept. 15, 2016, 8:53 p.m.) Review request for Default. Repository: gem5

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-15 Thread Jason Lowe-Power
> 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-15 Thread Andreas Hansson
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review8728 --- src/arch/riscv/RiscvISA.py (line 3)

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-15 Thread Alec Roelke
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/ --- (Updated Sept. 15, 2016, 8:21 p.m.) Review request for Default. Repository: gem5

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-15 Thread Alec Roelke
> 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-15 Thread Jason Lowe-Power
> On Sept. 15, 2016, 2:50 p.m., Jason Lowe-Power wrote: > > ext/libelf/elf_common.h, line 176 > > > > > > It would be nice if this were aligned with the above. > > Alec Roelke wrote: > Do you mean the two lines

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-15 Thread Jason Lowe-Power
> 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-15 Thread Alec Roelke
> 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-15 Thread Alec Roelke
--- 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

Re: [gem5-dev] Review Request 3624: arch: [Patch 1/5] Added RISC-V base instruction set RV64I

2016-09-15 Thread Jason Lowe-Power
--- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3624/#review8722 --- Thanks for the contribution. It looks pretty good! Could you give us a