On May 1, 2014, 8:08 p.m., Steve Reinhardt wrote:
Thanks! Interesting idea to generate a class and then derive from it.
Looks reasonable to me.
The comment called out below got me to wondering though... why did it
already talk about wrapping the enum in a class/struct when the
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2246/#review5082
---
Overall this looks pretty interesting. Some minor quibbles below, but
On May 4, 2014, 4:56 p.m., Steve Reinhardt wrote:
src/python/m5/simulate.py, line 117
http://reviews.gem5.org/r/2246/diff/1/?file=39626#file39626line117
for all these 'if hasattr()' changes (here and below, and in
simulate.py): seems like it would be much cleaner to make sure
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2247/#review5083
---
I didn't dig through this in detail, but it doesn't look any more
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2248/#review5085
---
Are all these changes really related to the ISA split? Some look like
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2249/#review5086
---
If these changes are necessary for the build system to work with the ISA
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2250/#review5087
---
Ship it!
Ship It!
- Steve Reinhardt
On April 23, 2014, 5:24 a.m.,
On May 5, 2014, 8:44 a.m., Andreas Hansson wrote:
It was my intention for all of this to be all required for ISA splitting;
these changes just address one aspect of the solution. If one single
changeset is desired, this should be folded into the other ISA splitting
changes.
I don't
On May 5, 2014, 8:44 a.m., Andreas Hansson wrote:
It was my intention for all of this to be all required for ISA splitting;
these changes just address one aspect of the solution. If one single
changeset is desired, this should be folded into the other ISA splitting
changes.
Steve
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2247/#review5093
---
Ship it!
Nice work!
- Steve Reinhardt
On May 7, 2014, 2:44 p.m.,
changeset cc6408469397 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=cc6408469397
description:
tests: update eio ref outputs for new stats
Also committed reference config.json files for
the eio tests.
diffstat:
changeset d51e31eef415 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=d51e31eef415
description:
tests: update t1000 pc-switcheroo-full stats
committed reference config.json files too
diffstat:
changeset 34f48d0dac97 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=34f48d0dac97
description:
syscall emulation: clean up comment SyscallReturn
diffstat:
src/arch/alpha/process.cc | 9 ++---
src/arch/arm/process.cc | 10 ++---
src/arch/mips/process.cc |
You could implement the setitimer syscall, but the bigger issue is that a
software sampling-based profiler like gprof isn't really appropriate to run
inside a simulator like gem5. gprof counts on a long enough runtime to
gather a lot of samples, which will take forever to run on a simulator.
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2272/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2273/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2274/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2277/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2275/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2276/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
On May 14, 2014, 6:41 a.m., Nilay Vaish wrote:
gem5 aborts due to segmentation fault with this patch applied.
Program received signal SIGSEGV, Segmentation fault.
Sequencer::writeCallback (this=0x3c9d800, address=..., data=...,
externalHit=false, mach=MachineType_NUM,
On May 14, 2014, 7:55 a.m., Ali Saidi wrote:
This looks fine to me, but I'm going to guess Andreas will have a comment.
This patch fixes the lovelock you mentioned, correct?
Yea, getting rid of the '+ 1' in PacketQueue::scheduleSend is needed to do two
stores in one cycle, an the change
On Thu, May 15, 2014 at 2:43 PM, Nilay Vaish ni...@cs.wisc.edu wrote:
I am guessing you tested with all the patches applied. When I applied
just this patch and tried to boot Linux, I got the segmentation fault.
Actually I did test most of the patches individually, I think including
this
I support getting rid of the templating in O3 for sure. Templates were new
and shiny when we started writing some parts of M5 and we got a little
carried away ;-).
As far as patch reviewing, I'm guessing many of those 35k lines of diffs
are mind-numbingly repetitive, right? If Mitch can split
Hi Nilay,
I cannot reproduce this failure. The only regression test for
X86_MESI_Two_Level uses a simple timing CPU, so that passes OK, as do all
the other regressions, even when this is the only patch applied. I tried
this command line:
build/X86_MESI_Two_Level/gem5.opt
It's an ethernet address (for binding to ethernet NICs). Note that it
(confusingly) corresponds to the python param class called 'EthernetAddr',
which is why grepping just for 'EthAddr' makes it look less used than it is.
Maybe I'm not looking at the same code you are, but I don't see what
was wondering if
this class was never completed or if this is the intended usage.
Anthony Gutierrez
http://web.eecs.umich.edu/~atgutier
On Fri, May 23, 2014 at 2:11 AM, Steve Reinhardt via gem5-dev
gem5-dev@gem5.org wrote:
It's an ethernet address (for binding to ethernet NICs). Note
will probably put the switch model and some changes to EthAddr on the
reviewboard next week then.
Anthony Gutierrez
http://web.eecs.umich.edu/~atgutier
On Fri, May 23, 2014 at 11:05 AM, Steve Reinhardt via gem5-dev
gem5-dev@gem5.org wrote:
Well that's a very different question
I'm guessing this is an unintended side effect of the recent scons revamp
in http://repo.gem5.org/gem5/rev/be0e1724eb39. Curtis, can you or someone
else comment on that?
However, I never use 'scons -c' myself anyway. First, scons almost always
does a good job of figuring out dependencies, so a
Sounds good to me. I'm still running Ubuntu 12.04 on my home machine,
which has 4.6, so I wouldn't want to push any further than that though.
Steve
On Thu, May 29, 2014 at 10:49 AM, Anthony Gutierrez via gem5-dev
gem5-dev@gem5.org wrote:
I strongly support this.
Anthony Gutierrez
was
wondering
if
this class was never completed or if this is the intended usage.
Anthony Gutierrez
http://web.eecs.umich.edu/~atgutier
On Fri, May 23, 2014 at 2:11 AM, Steve Reinhardt via gem5-dev
gem5-dev@gem5.org wrote:
It's an ethernet
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2281/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2282/#review5124
---
Ship it!
Looks good other than typo in comment...
SConstruct
changeset cb2e6950956d in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=cb2e6950956d
description:
style: eliminate equality tests with true and false
Using '== true' in a boolean expression is totally redundant,
and using '== false' is pretty verbose
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2279/#review5125
---
Nice! I didn't have time to read all the code closely, but I did notice
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2291/#review5143
---
I see you're talking about the 'secure' flag... wouldn't it make more
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2292/#review5144
---
Why is Addr being parsed using toMemorySize() in the first place? That
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2295/#review5145
---
src/cpu/o3/iew.hh
http://reviews.gem5.org/r/2295/#comment4669
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2302/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2276/
---
(Updated June 21, 2014, 10 a.m.)
Review request for Default.
Changes
---
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2298/#review5146
---
src/cpu/pred/2bit_local.hh
http://reviews.gem5.org/r/2298/#comment4670
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2294/#review5147
---
Ship it!
Minor comment below is just optional.
src/arch/isa_parser.py
On June 5, 2014, 3:32 a.m., Fernando Endo wrote:
Possibly a similar modification can be done in the rename stage. It stalls
the pipe if the LSQ is predicted to be full. This prediction is based in
the worst case (worst case of LQ and SQ entries, and considering that only
memory refs
changeset cb4e86c17767 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=cb4e86c17767
description:
stats: update for O3 changes
Mostly small differences in total ticks, but O3 stall causes
shifted significantly.
30.eon does speed up by ~6% on
, Steve Reinhardt via gem5-dev
gem5-dev@gem5.org wrote:
changeset cb4e86c17767 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=cb4e86c17767
description:
stats: update for O3 changes
Mostly small differences in total ticks, but O3 stall causes
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2303/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2295/#review5149
---
Ship it!
Ship It!
- Steve Reinhardt
On June 24, 2014, 7:19 a.m., Ali
On June 25, 2014, 7 a.m., Andreas Hansson wrote:
Perhaps I am missing something, but why would Ruby forward the packet to
the iobus in the first place if the address is not valid?
Steve Reinhardt wrote:
This is an FS thing, where you misspeculate in the kernel and generate a
Give me a chance... I haven't had time to go over Andrew's responses
carefully.
I appreciate the changes you've made so far, Andrew. I'm still not
convinced that Ticked needs to be a separate class though. What's the
point of an interface class when the only use of the interface is in
) the single instance of TickedObject in
Minor.
- Andrew
-Original Message-
From: gem5-dev [mailto:gem5-dev-boun...@gem5.org] On Behalf Of Steve
Reinhardt via gem5-dev
Sent: 01 July 2014 17:55
To: gem5 Developer List
Cc: Ali Saidi
Subject: Re: [gem5-dev] Review Request 2279: cpu
On June 4, 2014, 1:15 p.m., Steve Reinhardt wrote:
src/cpu/minor/cpu.hh, line 88
http://reviews.gem5.org/r/2279/diff/1/?file=39833#file39833line88
I don't know why, and I know it's not in the style guide, but all the
gem5 code I've seen (or written) has the colon at the beginning
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2304/#review5174
---
Ship it!
src/base/inet.hh
http://reviews.gem5.org/r/2304/#comment4689
On May 4, 2014, 4:56 p.m., Steve Reinhardt wrote:
src/python/m5/params.py, line 84
http://reviews.gem5.org/r/2246/diff/1/?file=39625#file39625line84
this seems clunky... would it be easier to list the classes that
*can't* be set on the command line?
Geoffrey Blake wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2306/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2307/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
Crazy! Thanks for tracking this down.
Steve
On Mon, Jul 7, 2014 at 10:15 AM, Anthony Gutierrez via gem5-dev
gem5-dev@gem5.org wrote:
Found the problem. This happens because the NSGigE Ethernet controller's
packet filter relies on the overloaded == operator for comparing two
EthAddr
On May 4, 2014, 4:56 p.m., Steve Reinhardt wrote:
src/python/m5/SimObject.py, line 654
http://reviews.gem5.org/r/2246/diff/1/?file=39624#file39624line654
This seems kind of complicated, but I'm going to wait for a
higher-level description of what it's doing before I dig into it.
changeset 00086092d9f7 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=00086092d9f7
description:
syscall emulation: fix DPRINTF arg ordering bug
When we switched getSyscallArg() from explicit arg indices to
the implicit method, some DPRINTF arguments
changeset f2de979b7bb3 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=f2de979b7bb3
description:
kern: get rid of unused linux syscall files
diffstat:
src/kern/SConscript |1 -
src/kern/linux/linux_syscalls.cc | 376
changeset b71e6c577768 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=b71e6c577768
description:
sim: remove unused MemoryModeStrings array
The System object has a static MemoryModeStrings array
that's (1) unused and (2) redundant, since there's an
changeset 563696c791d2 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=563696c791d2
description:
syscall emulation: fix fast build issue
Surprisingly gcc will complain about unused variables even
inside an 'if (false)' block.
I thought I had
changeset 23384aa97d85 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=23384aa97d85
description:
stats: update for syscall DPRINTF change
Only printing one rather than two args for the ignored syscall
warning means the count of register accesses has
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2167/#review5214
---
Hi Tony,
Sorry for taking so long to get around to this. I think your
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2167/#review5224
---
Ship it!
Looks good! Thanks for the changes.
- Steve Reinhardt
On
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2339/#review5270
---
This looks like a step in the right direction, but how about taking
On Aug. 18, 2014, 10:03 a.m., Nilay Vaish wrote:
I am fine with the patch as such. I think we should either maintain the
dependencies
here in this file or on the website. I don't see why we would like
maintain them in
two different places unless we can link them so that one
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2292/#review5301
---
Ship it!
Looks great, thanks!
- Steve Reinhardt
On Aug. 22, 2014,
Thanks for this, Andreas! I've tried to use it in the past and it never
did what I wanted... I assumed it was broken, not that it was working
correctly according to obscure semantics.
Is there any documentation for this beyond the brief 'usage' lines in the
script? It would be great if there
I don't know exactly what the problem is, but it's very suspicious that the
device would receive a cache-block-sized access when it's in uncacheable
memory space. My guess is that O3 is for some reason issuing a
misspeculated cacheable access to the physical address where the device
lives.
for all accesses in the PCI physical
address range and it fixed the problem.
Thanks,
Mohammad
On Friday, September 5, 2014 10:48 AM, Steve Reinhardt via gem5-dev
gem5-dev@gem5.org wrote:
I don't know exactly what the problem is, but it's very suspicious that the
device would receive
Tough to say... my gut feeling is that the numeric PC is more likely to be
correct, but on the other hand, the PC appears to map not only to a
different function, but to an instruction that's not even a memory access,
which makes it odd that it is generating a request packet.
Steve
On Tue, Sep
to memory, but I couldn't find anything.
Scott
On Wed, Sep 17, 2014 at 12:27 AM, Steve Reinhardt via gem5-dev
gem5-dev@gem5.org wrote:
Tough to say... my gut feeling is that the numeric PC is more likely to
be
correct, but on the other hand, the PC appears to map not only
.
Scott
On Wed, Sep 17, 2014 at 12:27 AM, Steve Reinhardt via gem5-dev
gem5-dev@gem5.org wrote:
Tough to say... my gut feeling is that the numeric PC is more likely
to
be
correct, but on the other hand, the PC appears to map not only to a
different function
, then there shouldn't be any of the instruction
stream in the output trace.
Scott
On Wed, Sep 17, 2014 at 11:58 AM, Steve Reinhardt via gem5-dev
gem5-dev@gem5.org wrote:
While we're on the topic, I'll just mention that there is a known bug
with
the gem5 symbol table: there's only one
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2378/#review5335
---
looks fine otherwise, no need to re-post IMO
src/base/inifile.cc
changeset 2b1bb16fd3d0 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=2b1bb16fd3d0
description:
stats: update eio stats for recent changes
diffstat:
tests/quick/se/20.eio-short/ref/alpha/eio/simple-atomic/config.ini |9 +-
changeset d96740732a61 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=d96740732a61
description:
stats: update t1000 stats for recent changes
diffstat:
tests/long/fs/80.solaris-boot/ref/sparc/solaris/t1000-simple-atomic/config.ini
| 26 +-
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2409/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2410/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2411/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2412/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2413/
---
Review request for Default.
Repository: gem5
Description
---
Changeset
On Sept. 24, 2014, 12:10 a.m., Andreas Hansson wrote:
src/sim/syscallreturn.hh, line 87
http://reviews.gem5.org/r/2413/diff/1/?file=41619#file41619line87
const?
Ah yes, I need an emacs macro that automatically puts in 'const' when I declare
a method, and then I can edit it out
On Sept. 24, 2014, 4:09 a.m., Andreas Hansson wrote:
src/sim/syscall_emul_buf.hh, line 73
http://reviews.gem5.org/r/2411/diff/1/?file=41616#file41616line73
Are there classes inheriting from this? If so, gcc 4.7 and beyond will
be unhappy about removing the virtual keyword for the
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2424/#review5382
---
Ship it!
Ship It!
- Steve Reinhardt
On Sept. 29, 2014, 3:36 a.m.,
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2427/#review5383
---
Ship it!
Ship It!
- Steve Reinhardt
On Sept. 29, 2014, 3:37 a.m.,
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2425/#review5385
---
SConstruct
http://reviews.gem5.org/r/2425/#comment4884
can we
On Sept. 30, 2014, 12:57 p.m., Andreas Hansson wrote:
Should this not be folded into the original patch?
Alexandru Dutu wrote:
I am not sure, that patch has already been commited. I thought once
commited, a patch becomes a commit and one can't modify it.
Andreas Hansson wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2448/#review5399
---
Ship it!
src/python/m5/params.py
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2442/#review5400
---
Are the bool cast operators being used anywhere? It looks like you put
Hi Ahmed,
Thanks for your willingness to attack this. We're glad to help out to the
extent that we can.
The documentation on the wiki is a bit of a mess at this point; there was
an effort to reorganize it that got stalled partway through, so sometimes
there are two copies of certain
I don't think there's a detailed list of what's implemented and what's
not. If you search in x86/isa/decoder for WarnUnimpl and FailUnimpl you'll
see quite a few instructions listed. Generally we implement instructions
as we run into the need for them though, so it may not be worth the effort
to
On Oct. 1, 2014, 4:37 p.m., Steve Reinhardt wrote:
Are the bool cast operators being used anywhere? It looks like you put
them in, but then do the 'ptr != nullptr' thing everywhere we need a bool,
so I'm wondering if they're useful.
How about replacing the cast operator with 'bool
On June 21, 2013, 1:44 p.m., Steve Reinhardt wrote:
This looks very good to me... I'm pleasantly surprised by how modest the
changes are. In addition to addressing the issues that Andrew brought up,
I suggest:
1. Adding a few high-level comments describing what's being done. I'm
changeset 4e715fe2abbd in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=4e715fe2abbd
description:
syscall_emul: minor style fix to LiveProcess constructor
diffstat:
src/sim/process.cc | 11 ---
1 files changed, 4 insertions(+), 7 deletions(-)
diffs (25
changeset 0a5a8ecd0ec6 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=0a5a8ecd0ec6
description:
syscall_emul: add EmulatedDriver object
Fake SE-mode device drivers can now be added by
deriving from this abstract object.
diffstat:
changeset 91b05b34b074 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=91b05b34b074
description:
syscall_emul: devirtualize BaseBufferArg methods
Not clear why they were marked virtual to begin with,
but that doesn't appear to be necessary.
diffstat:
changeset 5e0a421e2031 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=5e0a421e2031
description:
syscall_emul: add retry flag to SyscallReturn
This hook allows blocking emulated system calls to indicate
that they would block, but return control to the
changeset 73a59d5e0923 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=73a59d5e0923
description:
syscall_emul: Put BufferArg classes in a separate header.
Move the BufferArg classes that support syscall buffer args
(i.e., pointers into simulated user
Thanks to everyone, particularly Joel, for the discussion.
Joel makes a good case for having the option of a second copy of memory for
debugging and development purposes, and at first glance, Nilay's patch
seems reasonable to me. However, the backing store idea enforces a
sequentially consistent
1 - 100 of 245 matches
Mail list logo
