Re: [gem5-dev] Review Request: ISA parser: Simplify operand type handling.

[Steve Reinhardt]

On Sun, Jun 12, 2011 at 1:05 AM, Gabe Black  wrote:
>
> I was thinking about this today, and if we expand the read/write
> functions to handle signed types too, we're really just expanding the
> arbitrary set of types they can handle, not removing the limitation that
> you have to stay within those types which is what I think you don't like.

I think originally we supported memory accesses for any operand type
you could define, but that stopped being true once you made the
definitions extensible.  My immediate concern is just to make sure
that switching from the old explicit sign extensions to some implicit
sign extensions that happened as a side effect of C type conversions
is really doing the right thing, but having a cleaner way of doing
memory accesses of arbitrary types is a good idea.

> Instead of extending what we already have as far as explicit
> instantiation, it would be nice to have a more automatic mechanism where
> we'd just feed a list of types and a template (you can pass templates as
> template arguments, sort of like function pointers but for templates)
> and have some widget that cranks out the actual instantiation without so
> much copy and paste coding.

That sounds interesting, but seems like overkill... I just looked at
the SimpleCPU code, and as far as I can tell, the memory access type
(the arg type for read() and write()) is only used for two things: to
determine the size of the access, and to control the data type in the
InstRecord type for exec tracing (basically this is mostly setting the
data_status enum, but also using the proper double vs int field in the
data union).  The actual type clearly doesn't matter at all for the
first, and only a subset of types are supported for the second.

The original idea with the templates was that they might permit faster
implementations for functional CPU models that communicated directly
with memory.  However, if anything we've gone in the other direction
by implementing these templates in terms of readBytes() and
writeBytes().

So my general feeling is that if we want to make significant changes
to this interface, I'd be more inclined to streamline it and have the
generated ISA code call readBytes() and writeBytes() directly with a
size and some additional info to make exec tracing work (which should
get rid of the templates entirely, I think) rather than expanding the
template interface.  Then the burden of converting from an untyped
sequence of bytes to whatever the ISA wants could be done entirely in
the ISA definition, which seems like a good place for it.  Does that
make sense?  Do you think it's feasible or worthwhile?

> Also, while looking for information about Boost (in progress right now)
> I found their page where they talk about their license (link below).
> Looking through it, there are some ideas there which seem relevant to
> gem5. Specifically, I like the idea of a single license for everything,
> perhaps involving assigning copyright to a neutral body like a gem5
> foundation or something, and then just referring to it in the actual
> source files. That would get rid of lots of redundant text, and they
> make a good point that all that text is the sort of thing lawyers might
> get their underwear in a bunch over. There may be (but isn't
> necessarily) subtle variation on a file by file basis, and it's probably
> a lot more work to go through if somebody ever needed to do that.

We discussed this a long long time ago (when we first started
distributing the code, IIRC), and while it does have the advantages
you describe, the cost of further wrangling with lawyers is basically
not worth it IMO.  Maybe if we started a new project from scratch we
would consider it, but the horse has left the barn for gem5, I think.

Steve
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: LibElf: Build the error management code in libelf.

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/735/#review1326
---

Ship it!


- Steve


On 2011-06-04 11:39:02, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/735/
> ---
> 
> (Updated 2011-06-04 11:39:02)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> LibElf: Build the error management code in libelf.
> 
> This change makes some minor changes to get the error management code in
> libelf to build on Linux and to built it into the library.
> 
> 
> Diffs
> -
> 
>   ext/libelf/SConscript b9ba22cb23f2 
>   ext/libelf/_libelf.h b9ba22cb23f2 
>   ext/libelf/elf_errmsg.c b9ba22cb23f2 
> 
> Diff: http://reviews.m5sim.org/r/735/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabe
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: Loader: Handle bad section names when loading an ELF file.

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/736/#review1325
---

Ship it!


- Steve


On 2011-06-04 11:42:05, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/736/
> ---
> 
> (Updated 2011-06-04 11:42:05)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> Loader: Handle bad section names when loading an ELF file.
> 
> If there's a problem when reading the section names from a supposed ELF file,
> this change makes gem5 print an error message as returned by libelf and die.
> Previously these sorts of errors would make gem5 segfault when it tried to
> access the section name through a NULL pointer.
> 
> 
> Diffs
> -
> 
>   src/base/loader/elf_object.cc b9ba22cb23f2 
> 
> Diff: http://reviews.m5sim.org/r/736/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabe
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: inorder/dtb: make sure DTB translate correct address

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/743/#review1323
---


Shouldn't you get rid of the cache_unit.cc changes now?  I thought that was the 
point.

This is still a hack, in my opinion; note that the comment on the _pc field in 
mem/request.hh says "for tracing/debugging", i.e., it's not intended to be 
architectural.  Also, it isn't always set (e.g., for device accesses), though 
for CPU accesses it should be.  So I'd say (1) it should work and (2) it's a 
much less ugly hack than what you had before, so assuming you do get rid of the 
cache_unit.cc changes I'd say it's fine.

I still think having a ProxyThreadContext wrapped around a DynInst is the 
"right" way to do it, but I can see where that also looks like a lot of mostly 
unnecessary overhead.

- Steve


On 2011-06-10 22:52:04, Korey Sewell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/743/
> ---
> 
> (Updated 2011-06-10 22:52:04)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> inorder/dtb: make sure DTB translate correct address
> The DTB expects the correct PC in the ThreadContext
> but how if the memory accesses are speculative? Shouldn't
> we send along the requestor's PC to the translate functions?
> 
> 
> Diffs
> -
> 
>   src/arch/alpha/tlb.cc 77d12d8f7971 
>   src/cpu/inorder/resources/cache_unit.cc 77d12d8f7971 
> 
> Diff: http://reviews.m5sim.org/r/743/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Korey
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: cpus/isa: add a != operator for pcstate

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/738/#review1322
---

Ship it!


- Steve


On 2011-06-10 22:38:50, Korey Sewell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/738/
> ---
> 
> (Updated 2011-06-10 22:38:50)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> cpus/isa: add a != operator for pcstate
> 
> 
> Diffs
> -
> 
>   src/arch/arm/types.hh 77d12d8f7971 
>   src/arch/generic/types.hh 77d12d8f7971 
> 
> Diff: http://reviews.m5sim.org/r/738/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Korey
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Cron /z/m5/regression/do-regression quick

[Steve Reinhardt]

The regression runs as the 'm5test' user; if you tried to run under
/z/m5/regression as yourself or as root then it's probably running
into file/dir permissions problems.  I'll delete the build dir so we
shouldn't run into this again.

Usually it's best just to run the regressions in your own directory;
if you can run on zizzer then the regressions should run too.  You can
diff your tree with the one in /z/m5/regression/zizzer if necessary.
You don't have to wait more than 24 hours to see if the real
regression will work.

If you really need to rerun a regression the way cron does, you should use
sudo -u m5test -H /z/m5/regression/do-regression 
where  is either "quick" (what the nightly runs do) or
"--scratch all" (what happens Sat nights).

Steve

On Fri, Jun 10, 2011 at 1:16 AM, Korey Sewell  wrote:
> I was late in updating the repository. I think this may have happened since
> I was running stuff on zizzer while the regressions were loading up.  What's
> the method of choice for rerunning the do-regression script?
>
> Also, when updating the simple cpu regressions, I had to "hg merge" the
> changesets, so it seems I had a merge that didnt propagate through.
>
> I'm finally able to regenerate the o3-timing error seen in the regressions.
> I wont be able to fix those 2 regressions just this second, but the
> simple-cpu ones should be updated and when I get into the lab today, I'll
> look again into the O3 ones and see what happened.
>
>
> On Fri, Jun 10, 2011 at 3:02 AM, Cron Daemon 
> wrote:
>
>> scons: *** Cannot duplicate `src/SConscript' in `build/SPARC_SE': None.
>>  Stop.
>>
>> See /z/m5/regression/regress-2011-06-10-03:00:01 for details.
>>
>> ___
>> gem5-dev mailing list
>> gem5-dev@m5sim.org
>> http://m5sim.org/mailman/listinfo/gem5-dev
>>
>
>
>
> --
> - Korey
> ___
> gem5-dev mailing list
> gem5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/gem5-dev
>
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Cron /z/m5/regression/do-regression quick

[Steve Reinhardt]

It fails for me... are you sure you've pushed everything?  Have you
tried it on zizzer?

Steve

On Thu, Jun 9, 2011 at 1:36 PM, Korey Sewell  wrote:
> ok, this is a bit wierd.
>
> I'm running all the tests locally and they are passing ...
>
> Even the O3 one:
> build/SPARC_SE/tests/opt/quick/40.m5threads-test-atomic/sparc/linux/o3-timing-mp
>
> On Thu, Jun 9, 2011 at 1:52 PM, Korey Sewell  wrote:
>
>> Yup, that's me. I will update the stats for the simple cpus. I thought I
>> had caught the branchTarget() error before, but apparently not.
>>
>>
>> On Thu, Jun 9, 2011 at 1:45 PM, Steve Reinhardt  wrote:
>>
>>> Looks like all the SPARC tests failed.  The two o3-timing ones have this
>>> error:
>>>
>>> panic: StaticInst::branchTarget() called on instruction that is not a
>>> PC-relative branch.
>>> [branchTarget:build/SPARC_SE/cpu/static_inst.cc, line 99]
>>>
>>> The others seem to have run correctly, but have stats differences like
>>> this:
>>>
>>>  system.cpu.num_conditional_control_insts          0        774
>>>  774  +.00%
>>>  system.cpu.num_func_calls               0        146        146
>>>  +.00%
>>>
>>> I'm guessing this is from Korey's recent SPARC change...
>>>
>>> Steve
>>>
>>>
>>> On Thu, Jun 9, 2011 at 12:46 AM, Cron Daemon 
>>> wrote:
>>> > *
>>> build/SPARC_SE/tests/opt/quick/40.m5threads-test-atomic/sparc/linux/o3-timing-mp
>>> FAILED!
>>> > * build/SPARC_SE/tests/opt/quick/02.insttest/sparc/linux/o3-timing
>>> FAILED!
>>> > *
>>> build/SPARC_SE/tests/opt/quick/02.insttest/sparc/linux/simple-atomic FAILED!
>>> > *
>>> build/SPARC_SE/tests/opt/quick/40.m5threads-test-atomic/sparc/linux/simple-atomic-mp
>>> FAILED!
>>> > *
>>> build/SPARC_SE/tests/opt/quick/02.insttest/sparc/linux/simple-timing FAILED!
>>> > *
>>> build/SPARC_SE/tests/opt/quick/40.m5threads-test-atomic/sparc/linux/simple-timing-mp
>>> FAILED!
>>> > *
>>> build/SPARC_SE/tests/opt/quick/00.hello/sparc/linux/simple-timing-ruby
>>> FAILED!
>>> > * build/SPARC_SE/tests/opt/quick/00.hello/sparc/linux/simple-atomic
>>> FAILED!
>>> > * build/SPARC_SE/tests/opt/quick/00.hello/sparc/linux/simple-timing
>>> FAILED!
>>> > scons: *** Found dependency cycle(s):
>>> > *
>>> build/ALPHA_SE/tests/opt/quick/30.eio-mp/alpha/eio/simple-atomic-mp passed.
>>> > *
>>> build/ALPHA_SE/tests/opt/quick/20.eio-short/alpha/eio/simple-atomic passed.
>>> > * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/inorder-timing
>>> passed.
>>> > * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/simple-timing
>>> passed.
>>> > * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/o3-timing
>>> passed.
>>> > *
>>> build/ALPHA_SE/tests/opt/quick/00.hello/alpha/tru64/simple-timing-ruby
>>> passed.
>>> > *
>>> build/ALPHA_SE/tests/opt/quick/20.eio-short/alpha/eio/simple-timing passed.
>>> > *
>>> build/ALPHA_SE/tests/opt/quick/30.eio-mp/alpha/eio/simple-timing-mp passed.
>>> > *
>>> build/ALPHA_SE/tests/opt/quick/60.rubytest/alpha/linux/rubytest-ruby passed.
>>> > * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/tru64/simple-timing
>>> passed.
>>> > * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/tru64/simple-atomic
>>> passed.
>>> > * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/simple-atomic
>>> passed.
>>> > *
>>> build/ALPHA_SE_MOESI_hammer/tests/opt/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_hammer
>>> passed.
>>> > *
>>> build/ALPHA_SE_MOESI_hammer/tests/opt/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_hammer
>>> passed.
>>> > *
>>> build/ALPHA_SE_MOESI_hammer/tests/opt/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_hammer
>>> passed.
>>> > *
>>> build/ALPHA_SE/tests/opt/quick/01.hello-2T-smt/alpha/linux/o3-timing passed.
>>> > * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/tru64/o3-timing
>>> passed.
>>> > *
>>> build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/simple-timing-ruby
>>> passed.
>>> > *
>>> build/ALPHA_SE_MESI_CMP_directory/tests

Re: [gem5-dev] Review Request: inorder/dtb: make sure DTB translate correct address

[Steve Reinhardt]

> On 2011-06-09 11:17:24, Gabe Black wrote:
> > This isn't a review, just a thought on the question you're asking. If the 
> > access is speculative, is it ok to use a misspeculated pc since the 
> > instruction will be thrown out anyway?

Actually after briefly looking at the code, I wonder if we don't have a bigger 
problem here.  I'm assuming that the ThreadContext struct reflects the 
committed state of the machine, where we really want the state relative to this 
in-flight instruction.  Have we just been getting lucky so far?  Seems like in 
Alpha the TLB only uses the PC to see if it's in PAL mode or not, so maybe the 
pipeline gets flushed when we switch in and out of PAL mode and thus there's 
never an inconsistency there even in O3.  (Nate or Ali, can you confirm this?)

Korey, can you elaborate on the kind of errors you were running into without 
this change?

Technically I think perhaps the TLB should be using the ExecContext interface 
instead of the ThreadContext, but since ExecContext isn't a real object, that 
only works if you template the call... I wonder if we really should be using 
something like the ProxyThreadContext (see cpu/thread_context.hh) wrapped 
around a DynInst.  Does that make sense?


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/743/#review1311
---


On 2011-06-08 23:34:50, Korey Sewell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/743/
> ---
> 
> (Updated 2011-06-08 23:34:50)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> inorder/dtb: make sure DTB translate correct address
> The DTB expects the correct PC in the ThreadContext
> but how if the memory accesses are speculative? Shouldn't
> we send along the requestor's PC to the translate functions?
> 
> 
> Diffs
> -
> 
>   src/cpu/inorder/resources/cache_unit.cc 77d12d8f7971 
> 
> Diff: http://reviews.m5sim.org/r/743/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Korey
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: cpus/isa: add a != operator for pcstate

[Steve Reinhardt]

> On 2011-06-08 22:52:15, Gabe Black wrote:
> > I think you missed some (maybe just one) version of PC state defined in the 
> > ISAs themselves. ARM may be the only one, but you should double check to be 
> > sure. Also, for all these you could define them using ==, something like 
> > return !(*this == opc);
> 
> Korey Sewell wrote:
> The "!(*this == opc)" is interesting. If you do it that way, it's 
> definitely more programmable for the long term since one change to the equals 
> operator definition will propagate. But, is that way adding two extra 
> operations there? 1 to dereference the this pointer and then another to do 
> the NOT operation? The "==" operator is the more used operator but if for 
> some reason the "!=" operator become popular within gem5 wouldnt it be 
> slightly slower?
> 
> Steve Reinhardt wrote:
> I think defining != in terms of == is much preferred for the reasons you 
> said.  The compiler should inline the == definition into != where 
> appropriate, so the performance difference for optimized code should be 
> somewhere between non-existent and negligible, and certainly worthwhile given 
> how much simpler and more robust the code will be.  You should just be able 
> to define it once on the base class too which will simplify things even more.
> 
> Gabe Black wrote:
> Be really careful just defining it in the base class. It's not virtual as 
> far as I remember, and generally speaking the subclasses add extra things to 
> check. You could end up using the base class version and not checking 
> everything, but it would frequently be right and could slip by pretty easily. 
> I'm not saying it won't work, just be very careful and verify it's doing 
> exactly what you think it is in all the ISAs one by one.

You're right, it doesn't work to just define it in the base class since the 
other functions are not virtual.  So forget my comment on that one, you will 
have to define it in each class where == is defined.  Defining != in terms of 
== is still the way to go though.


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/738/#review1303
---


On 2011-06-08 22:46:05, Korey Sewell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/738/
> ---
> 
> (Updated 2011-06-08 22:46:05)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> cpus/isa: add a != operator for pcstate
> 
> 
> Diffs
> -
> 
>   src/arch/generic/types.hh 77d12d8f7971 
> 
> Diff: http://reviews.m5sim.org/r/738/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Korey
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Cron /z/m5/regression/do-regression quick

[Steve Reinhardt]

Looks like all the SPARC tests failed.  The two o3-timing ones have this error:

panic: StaticInst::branchTarget() called on instruction that is not a
PC-relative branch.
[branchTarget:build/SPARC_SE/cpu/static_inst.cc, line 99]

The others seem to have run correctly, but have stats differences like this:

  system.cpu.num_conditional_control_insts  0774
 774  +.00%
  system.cpu.num_func_calls   0146146  +.00%

I'm guessing this is from Korey's recent SPARC change...

Steve


On Thu, Jun 9, 2011 at 12:46 AM, Cron Daemon  wrote:
> * 
> build/SPARC_SE/tests/opt/quick/40.m5threads-test-atomic/sparc/linux/o3-timing-mp
>  FAILED!
> * build/SPARC_SE/tests/opt/quick/02.insttest/sparc/linux/o3-timing FAILED!
> * build/SPARC_SE/tests/opt/quick/02.insttest/sparc/linux/simple-atomic 
> FAILED!
> * 
> build/SPARC_SE/tests/opt/quick/40.m5threads-test-atomic/sparc/linux/simple-atomic-mp
>  FAILED!
> * build/SPARC_SE/tests/opt/quick/02.insttest/sparc/linux/simple-timing 
> FAILED!
> * 
> build/SPARC_SE/tests/opt/quick/40.m5threads-test-atomic/sparc/linux/simple-timing-mp
>  FAILED!
> * build/SPARC_SE/tests/opt/quick/00.hello/sparc/linux/simple-timing-ruby 
> FAILED!
> * build/SPARC_SE/tests/opt/quick/00.hello/sparc/linux/simple-atomic 
> FAILED!
> * build/SPARC_SE/tests/opt/quick/00.hello/sparc/linux/simple-timing 
> FAILED!
> scons: *** Found dependency cycle(s):
> * build/ALPHA_SE/tests/opt/quick/30.eio-mp/alpha/eio/simple-atomic-mp 
> passed.
> * build/ALPHA_SE/tests/opt/quick/20.eio-short/alpha/eio/simple-atomic 
> passed.
> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/inorder-timing 
> passed.
> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/simple-timing 
> passed.
> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/o3-timing passed.
> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/tru64/simple-timing-ruby 
> passed.
> * build/ALPHA_SE/tests/opt/quick/20.eio-short/alpha/eio/simple-timing 
> passed.
> * build/ALPHA_SE/tests/opt/quick/30.eio-mp/alpha/eio/simple-timing-mp 
> passed.
> * build/ALPHA_SE/tests/opt/quick/60.rubytest/alpha/linux/rubytest-ruby 
> passed.
> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/tru64/simple-timing 
> passed.
> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/tru64/simple-atomic 
> passed.
> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/simple-atomic 
> passed.
> * 
> build/ALPHA_SE_MOESI_hammer/tests/opt/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_hammer
>  passed.
> * 
> build/ALPHA_SE_MOESI_hammer/tests/opt/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_hammer
>  passed.
> * 
> build/ALPHA_SE_MOESI_hammer/tests/opt/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_hammer
>  passed.
> * build/ALPHA_SE/tests/opt/quick/01.hello-2T-smt/alpha/linux/o3-timing 
> passed.
> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/tru64/o3-timing passed.
> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/simple-timing-ruby 
> passed.
> * 
> build/ALPHA_SE_MESI_CMP_directory/tests/opt/quick/60.rubytest/alpha/linux/rubytest-ruby-MESI_CMP_directory
>  passed.
> * 
> build/ALPHA_SE_MESI_CMP_directory/tests/opt/quick/00.hello/alpha/tru64/simple-timing-ruby-MESI_CMP_directory
>  passed.
> * 
> build/ALPHA_SE_MESI_CMP_directory/tests/opt/quick/00.hello/alpha/linux/simple-timing-ruby-MESI_CMP_directory
>  passed.
> * build/ALPHA_SE/tests/opt/quick/50.memtest/alpha/linux/memtest-ruby 
> passed.
> * build/ALPHA_SE/tests/opt/quick/50.memtest/alpha/linux/memtest passed.
> * 
> build/ALPHA_SE_MOESI_CMP_directory/tests/opt/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_directory
>  passed.
> * 
> build/ALPHA_SE_MOESI_CMP_directory/tests/opt/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_directory
>  passed.
> * 
> build/ALPHA_SE_MOESI_CMP_directory/tests/opt/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_directory
>  passed.
> * 
> build/ALPHA_SE_MOESI_CMP_token/tests/opt/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_token
>  passed.
> * 
> build/ALPHA_SE_MOESI_CMP_token/tests/opt/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_token
>  passed.
> * 
> build/ALPHA_SE_MOESI_CMP_token/tests/opt/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_token
>  passed.
> * 
> build/ALPHA_SE_MOESI_hammer/tests/opt/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_hammer
>  passed.
> * 
> build/ALPHA_FS/tests/opt/quick/10.linux-boot/alpha/linux/tsunami-simple-timing
>  passed.
> * 
> build/ALPHA_FS/tests/opt/quick/10.linux-boot/alpha/linux/tsunami-simple-timing-dual
>  passed.
> * 
> build/ALPHA_FS/tests/opt/quick/10.linux-boot/alpha/linux/tsunami-simple-atomic-dual
>  passed.
> * 
> build/ALPHA_FS/tests/opt/quick/10.linux-boot/alpha/linux/tsunami-simple-atomic
>  passed.
> * 
> build/ALPHA_FS/tests/opt/quick/80

Re: [gem5-dev] Review Request: cpus/isa: add a != operator for pcstate

[Steve Reinhardt]

> On 2011-06-08 22:52:15, Gabe Black wrote:
> > I think you missed some (maybe just one) version of PC state defined in the 
> > ISAs themselves. ARM may be the only one, but you should double check to be 
> > sure. Also, for all these you could define them using ==, something like 
> > return !(*this == opc);
> 
> Korey Sewell wrote:
> The "!(*this == opc)" is interesting. If you do it that way, it's 
> definitely more programmable for the long term since one change to the equals 
> operator definition will propagate. But, is that way adding two extra 
> operations there? 1 to dereference the this pointer and then another to do 
> the NOT operation? The "==" operator is the more used operator but if for 
> some reason the "!=" operator become popular within gem5 wouldnt it be 
> slightly slower?

I think defining != in terms of == is much preferred for the reasons you said.  
The compiler should inline the == definition into != where appropriate, so the 
performance difference for optimized code should be somewhere between 
non-existent and negligible, and certainly worthwhile given how much simpler 
and more robust the code will be.  You should just be able to define it once on 
the base class too which will simplify things even more.


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/738/#review1303
---


On 2011-06-08 22:46:05, Korey Sewell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/738/
> -------
> 
> (Updated 2011-06-08 22:46:05)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> cpus/isa: add a != operator for pcstate
> 
> 
> Diffs
> -
> 
>   src/arch/generic/types.hh 77d12d8f7971 
> 
> Diff: http://reviews.m5sim.org/r/738/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Korey
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: ISA parser: Loosen the regular expressions matching filenames.

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/737/#review1300
---

Ship it!


- Steve


On 2011-06-07 02:24:01, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/737/
> ---
> 
> (Updated 2011-06-07 02:24:01)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> ISA parser: Loosen the regular expressions matching filenames.
> 
> The regular expressions matching filenames in the ##include directives and the
> internally generated ##newfile directives where only looking for filenames
> composed of alpha numeric characters, periods, and dashes. In Unix/Linux, the
> rules for what characters can be in a filename are much looser than that. This
> change replaces those expressions with ones that look for anything other than
> a quote character. Technically quote characters are allowed as well so we
> should allow escaping them somehow, but the additional complexity probably
> isn't worth it.
> 
> 
> Diffs
> -
> 
>   src/arch/isa_parser.py 4d1005f78496 
> 
> Diff: http://reviews.m5sim.org/r/737/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabe
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] IMPORTANT: Repository URLs have changed

[Steve Reinhardt]

As part of our continued transition from m5 and GEMS to gem5, the URLs
for HTTP access to our repositories have changed from m5 and m5-stable
to gem5 and gem5-stable.  The full URLs for these repositories are
now:

http://repo.gem5.org/gem5
http://repo.gem5.org/gem5-stable

If you had previously cloned the m5 or m5-stable repository using
HTTP, in order to update your local repository, you will need to edit
the .hg/hgrc file in that repository to change the section:

[paths]
default = http://repo.m5sim.org/m5

to read:

[paths]
default = http://repo.gem5.org/gem5

(or similarly for m5-stable -> gem5-stable).

Note that the repository itself has not changed, only the URL to
access it via HTTP.

For developers who access the repository via ssh, symlinks have been
created so that you do not have to change your path immediately, but
it would be a good idea to start using the new gem5 name anyway.

Steve
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Cron /z/m5/regression/do-regression --scratch all

[Steve Reinhardt]

My fault... I moved the repo URL from m5 to gem5 and the regression
script couldn't find it anymore.  I'm re-running now.

Steve

On Sun, Jun 5, 2011 at 12:04 AM, Cron Daemon  wrote:
>
> See /z/m5/regression/regress-2011-06-05-03:00:01 for details.
>
> ___
> gem5-dev mailing list
> gem5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/gem5-dev
>
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Cron /z/m5/regression/do-regression quick

[Steve Reinhardt]

That will happen automatically tonight (the Sat night/Sun AM
regression runs with the --scratch flag, see /etc/cron.d/m5).

Steve

On Sat, Jun 4, 2011 at 11:22 AM, Nilay Vaish  wrote:
> Will clearing all the existing builds and starting afresh remove this issue?
> Can some one do this?
>
> --
> Nilay
>
> On Sat, 4 Jun 2011, Steve Reinhardt wrote:
>
>> It seems very strange... like at a high level it thinks there's a
>> cycle, but when it goes to print out where it is it can't find one.
>> I've never seen this myself; I wonder if it's a bug in the version of
>> scons on zizzer (v0.98), as the machine I use has v.1.2.0.
>>
>> It is a little strange that we're building params structs for Ruby
>> objects in ALPHA_SE though.
>>
>> Steve
>>
>> On Sat, Jun 4, 2011 at 6:41 AM, Nilay Vaish  wrote:
>>>
>>> Does any one has any idea what a dependency cycles is? This is what
>>> zizzer's
>>> log has.
>>>
>>> scons: *** Found dependency cycle(s):
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE/params/L1Cache_Controller.hh (>> at
>>> 0x412a680>) in state up_to_date
>>> Internal Error: no cycle found for node
>>> build/ALPHA_SE/params/Directory_Controller.hh (>> instance
>>> at 0x410d200>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MOESI_CMP_directory/params/L2Cache_Controller.hh
>>> () in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MESI_CMP_directory/params/L2Cache_Controller.hh
>>> () in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MOESI_CMP_token/params/DMA_Controller.hh
>>> (>> instance at 0x11ccb4d0>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/X86_SE/params/DMA_Controller.hh (>> 0x30ae9b90>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MOESI_hammer/params/Directory_Controller.hh
>>> () in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MOESI_CMP_directory/params/L1Cache_Controller.hh
>>> () in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_FS/params/Directory_Controller.hh (>> instance
>>> at 0x16ea8560>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ARM_SE/params/L1Cache_Controller.hh (>> at
>>> 0x3a89d518>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE/params/DMA_Controller.hh (>> 0x4105ef0>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MOESI_CMP_directory/params/Directory_Controller.hh
>>> () in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MOESI_CMP_token/params/L2Cache_Controller.hh
>>> () in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/X86_SE/params/L1Cache_Controller.hh (>> at
>>> 0x30be19e0>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/X86_FS/params/DMA_Controller.hh (>> 0x34b024d0>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/X86_FS/params/L1Cache_Controller.hh (>> at
>>> 0x34cfcc20>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MOESI_CMP_directory/params/DMA_Controller.hh
>>> () in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ARM_SE/params/DMA_Controller.hh (>> 0x3a7b16c8>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MOESI_hammer/params/DMA_Controller.hh (>> instance at 0x9b58248>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MESI_CMP_directory/params/Directory_Controller.hh
>>> () in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/X86_FS/params/Directory_Controller.hh (>> at
>>> 0x34b08ef0>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/SPARC_FS/params/Directory_Controller.hh (>> instance
>>> at 0x2c1b0f38>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MOESI_CMP_token/params/L1Cache_Controller.hh
>>> () in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_FS/params/L1Cache_Controller.hh (>> at
>>> 0x16ec7290>) in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MESI_CMP_directory/params/L1Cache_Controller.hh
>>> () in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MESI_CMP_directory/params/DMA_Controller.hh
>>> () in state up_to_date
>>>  Internal Error: no cycle found for node
>>> build/ALPHA_SE_MOESI_hammer/params/L1Cache_Controller.hh
>>> () in state up_to_date
>>>
>>> --
>>> Nilay
>>>
>
> ___
> gem5-dev mailing list
> gem5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/gem5-dev
>
>
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Cron /z/m5/regression/do-regression quick

[Steve Reinhardt]

It seems very strange... like at a high level it thinks there's a
cycle, but when it goes to print out where it is it can't find one.
I've never seen this myself; I wonder if it's a bug in the version of
scons on zizzer (v0.98), as the machine I use has v.1.2.0.

It is a little strange that we're building params structs for Ruby
objects in ALPHA_SE though.

Steve

On Sat, Jun 4, 2011 at 6:41 AM, Nilay Vaish  wrote:
> Does any one has any idea what a dependency cycles is? This is what zizzer's
> log has.
>
> scons: *** Found dependency cycle(s):
>  Internal Error: no cycle found for node
> build/ALPHA_SE/params/L1Cache_Controller.hh ( 0x412a680>) in state up_to_date
> Internal Error: no cycle found for node
> build/ALPHA_SE/params/Directory_Controller.hh ( at 0x410d200>) in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MOESI_CMP_directory/params/L2Cache_Controller.hh
> () in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MESI_CMP_directory/params/L2Cache_Controller.hh
> () in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MOESI_CMP_token/params/DMA_Controller.hh ( instance at 0x11ccb4d0>) in state up_to_date
>  Internal Error: no cycle found for node
> build/X86_SE/params/DMA_Controller.hh ( 0x30ae9b90>) in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MOESI_hammer/params/Directory_Controller.hh
> () in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MOESI_CMP_directory/params/L1Cache_Controller.hh
> () in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_FS/params/Directory_Controller.hh ( at 0x16ea8560>) in state up_to_date
>  Internal Error: no cycle found for node
> build/ARM_SE/params/L1Cache_Controller.hh ( 0x3a89d518>) in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE/params/DMA_Controller.hh ( 0x4105ef0>) in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MOESI_CMP_directory/params/Directory_Controller.hh
> () in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MOESI_CMP_token/params/L2Cache_Controller.hh
> () in state up_to_date
>  Internal Error: no cycle found for node
> build/X86_SE/params/L1Cache_Controller.hh ( 0x30be19e0>) in state up_to_date
>  Internal Error: no cycle found for node
> build/X86_FS/params/DMA_Controller.hh ( 0x34b024d0>) in state up_to_date
>  Internal Error: no cycle found for node
> build/X86_FS/params/L1Cache_Controller.hh ( 0x34cfcc20>) in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MOESI_CMP_directory/params/DMA_Controller.hh
> () in state up_to_date
>  Internal Error: no cycle found for node
> build/ARM_SE/params/DMA_Controller.hh ( 0x3a7b16c8>) in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MOESI_hammer/params/DMA_Controller.hh ( instance at 0x9b58248>) in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MESI_CMP_directory/params/Directory_Controller.hh
> () in state up_to_date
>  Internal Error: no cycle found for node
> build/X86_FS/params/Directory_Controller.hh ( 0x34b08ef0>) in state up_to_date
>  Internal Error: no cycle found for node
> build/SPARC_FS/params/Directory_Controller.hh ( at 0x2c1b0f38>) in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MOESI_CMP_token/params/L1Cache_Controller.hh
> () in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_FS/params/L1Cache_Controller.hh ( 0x16ec7290>) in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MESI_CMP_directory/params/L1Cache_Controller.hh
> () in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MESI_CMP_directory/params/DMA_Controller.hh
> () in state up_to_date
>  Internal Error: no cycle found for node
> build/ALPHA_SE_MOESI_hammer/params/L1Cache_Controller.hh
> () in state up_to_date
>
> --
> Nilay
>
>
> On Sat, 4 Jun 2011, Cron Daemon wrote:
>
>> scons: *** Found dependency cycle(s):
>> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/o3-timing
>> passed.
>> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/simple-timing
>> passed.
>> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/tru64/simple-atomic
>> passed.
>> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/tru64/simple-timing
>> passed.
>> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/inorder-timing
>> passed.
>> * build/ALPHA_SE/tests/opt/quick/60.rubytest/alpha/linux/rubytest-ruby
>> passed.
>> * build/ALPHA_SE/tests/opt/quick/00.hello/alpha/linux/simple-atomic
>> passed.
>> *
>> build/ALPHA_SE/tests/opt/quick/00.hello/alpha/tru64/simple-timing-ruby
>> passed.
>> *
>> build/ALPHA_SE_MOESI_hammer/tests/opt/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_hammer
>> passed.
>> *
>> build/ALPHA_SE_MOESI_hammer/tests/opt/quick/00.hello

Re: [gem5-dev] Review Request: ISA parser: Simplify operand type handling.

[Steve Reinhardt]

On Sat, Jun 4, 2011 at 1:57 AM, Gabe Black  wrote:
> To clarify, is this signed/unsigned issue something we need to deal with
> before this patch goes in, or can it be dealt with separately later?

I'd like to see it handled before the patch is committed, mostly
because I'm still not 100% convinced that these changes don't break
something.  Also when something gets deferred like this there's always
the uncertainty of when/if it's going to get taken care of... nothing
personal, I'd feel the same way if it was my own patch.

Steve
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in m5: SConstruct: automatically update .hg/hgrc with ...

[Steve Reinhardt]

changeset 3a2aebf01bf3 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=3a2aebf01bf3
description:
SConstruct: automatically update .hg/hgrc with style hooks.
Seems easier than pestering people about it.
Note also that path is now absolute, so you don't get errors
when invoking hg from subdirectories.
Also whacked unused mercurial_bin_not_found message (the
code that used this was deleted a couple months ago in
rev 5138d1e453f1).

diffstat:

 SConstruct |  72 +++--
 1 files changed, 41 insertions(+), 31 deletions(-)

diffs (98 lines):

diff -r 9228e00459d4 -r 3a2aebf01bf3 SConstruct
--- a/SConstructThu Jun 02 17:36:21 2011 -0700
+++ b/SConstructThu Jun 02 21:23:02 2011 -0700
@@ -199,53 +199,62 @@
 hgdir = main.root.Dir(".hg")
 
 mercurial_style_message = """
-You're missing the M5 style hook.
-Please install the hook so we can ensure that all code fits a common style.
+You're missing the gem5 style hook, which automatically checks your code
+against the gem5 style rules on hg commit and qrefresh commands.  This
+script will now install the hook in your .hg/hgrc file.
+Press enter to continue, or ctrl-c to abort: """
 
-All you'd need to do is add the following lines to your repository .hg/hgrc
-or your personal .hgrc
-
-
+mercurial_style_hook = """
+# The following lines were automatically added by gem5/SConstruct
+# to provide the gem5 style-checking hooks
 [extensions]
 style = %s/util/style.py
 
 [hooks]
 pretxncommit.style = python:style.check_style
 pre-qrefresh.style = python:style.check_style
-""" % (main.root)
+# End of SConstruct additions
 
-mercurial_bin_not_found = """
-Mercurial binary cannot be found, unfortunately this means that we
-cannot easily determine the version of M5 that you are running and
-this makes error messages more difficult to collect.  Please consider
-installing mercurial if you choose to post an error message
+""" % (main.root.abspath)
+
+mercurial_lib_not_found = """
+Mercurial libraries cannot be found, ignoring style hook.  If
+you are a gem5 developer, please fix this and run the style
+hook. It is important.
 """
 
-mercurial_lib_not_found = """
-Mercurial libraries cannot be found, ignoring style hook
-If you are actually a M5 developer, please fix this and
-run the style hook. It is important.
-"""
-
-if hgdir.exists():
-# Ensure that the style hook is in place.
+# Check for style hook and prompt for installation if it's not there.
+# Skip this if --ignore-style was specified, there's no .hg dir to
+# install a hook in, or there's no interactive terminal to prompt.
+if not GetOption('ignore_style') and hgdir.exists() and sys.stdin.isatty():
+style_hook = True
 try:
-ui = None
-if not GetOption('ignore_style'):
-from mercurial import ui
-ui = ui.ui()
+from mercurial import ui
+ui = ui.ui()
+ui.readconfig(hgdir.File('hgrc').abspath)
+style_hook = ui.config('hooks', 'pretxncommit.style', None) and \
+ ui.config('hooks', 'pre-qrefresh.style', None)
 except ImportError:
 print mercurial_lib_not_found
 
-if ui is not None:
-ui.readconfig(hgdir.File('hgrc').abspath)
-style_hook = ui.config('hooks', 'pretxncommit.style', None)
+if not style_hook:
+print mercurial_style_message,
+# continue unless user does ctrl-c/ctrl-d etc.
+try:
+raw_input()
+except:
+print "Input exception, exiting scons.\n"
+sys.exit(1)
+hgrc_path = '%s/.hg/hgrc' % main.root.abspath
+print "Adding style hook to", hgrc_path, "\n"
+try:
+hgrc = open(hgrc_path, 'a')
+hgrc.write(mercurial_style_hook)
+hgrc.close()
+except:
+print "Error updating", hgrc_path
+sys.exit(1)
 
-if not style_hook:
-print mercurial_style_message
-sys.exit(1)
-else:
-print ".hg directory not found"
 
 ###
 #
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: copyright: clean up copyright blocks

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/725/#review1299
---

Ship it!


- Steve


On 2011-06-02 14:47:10, Nathan Binkert wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/725/
> ---
> 
> (Updated 2011-06-02 14:47:10)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> copyright: clean up copyright blocks
> 
> This is basically to make it easier to run the find_copyrights.py utility 
> that's added in the next changeset.  It fixes some consistency of naming.  
> (e.g. Advanced Micro Devices -> Advanced Micro Devices, Inc.)
> 
> 
> Diffs
> -
> 
>   src/arch/alpha/kgdb.h aa00cee9abb1 
>   src/arch/alpha/remote_gdb.cc aa00cee9abb1 
>   src/arch/arm/remote_gdb.cc aa00cee9abb1 
>   src/arch/generic/debugfaults.hh aa00cee9abb1 
>   src/arch/sparc/remote_gdb.cc aa00cee9abb1 
>   src/arch/x86/insts/badmicroop.hh aa00cee9abb1 
>   src/arch/x86/insts/badmicroop.cc aa00cee9abb1 
>   src/arch/x86/isa/formats/nop.isa aa00cee9abb1 
>   src/base/random_mt.cc aa00cee9abb1 
>   src/base/remote_gdb.cc aa00cee9abb1 
>   src/dev/ide_wdcreg.h aa00cee9abb1 
>   src/dev/sparc/dtod.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Allocator/Arbiter.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Allocator/Arbiter.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Allocator/MatrixArbiter.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Allocator/MatrixArbiter.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Allocator/RRArbiter.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Allocator/RRArbiter.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Allocator/SWAllocator.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Allocator/SWAllocator.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Allocator/VCAllocator.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Allocator/VCAllocator.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/AmpUnit.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/AmpUnit.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/BitlineUnit.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/BitlineUnit.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/Buffer.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/Buffer.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/DecoderUnit.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/DecoderUnit.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/MemUnit.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/MemUnit.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/OutdrvUnit.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/OutdrvUnit.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/PrechargeUnit.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/PrechargeUnit.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/Register.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/Register.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/SRAM.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/SRAM.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/WordlineUnit.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Buffer/WordlineUnit.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Clock.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Clock.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Crossbar/Crossbar.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Crossbar/Crossbar.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Crossbar/MatrixCrossbar.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Crossbar/MatrixCrossbar.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Crossbar/MultreeCrossbar.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Crossbar/MultreeCrossbar.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/FlipFlop.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/FlipFlop.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/OrionLink.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/OrionLink.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/OrionRouter.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/OrionRouter.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/TechParameter.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/TechParameter.cc aa00cee9abb1 
>   src/mem/ruby/network/orion/Type.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Wire.hh aa00cee9abb1 
>   src/mem/ruby/network/orion/Wire.cc aa00cee9abb1 
>   src/sim/fault_fwd

Re: [gem5-dev] Review Request: copyright: Add code for finding all copyright blocks and create a COPYING file

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/726/#review1298
---



src/python/m5/main.py
<http://reviews.m5sim.org/r/726/#comment1764>

This is printed out every time, right?  If so, it seems pretty verbose.  
How about just:
'gem5 is copyrighted software; use the --copyright option for details.'

If you want to work the URL in, I'd put it in the initial header, e.g.,
'gem5 Simulator System, http://gem5.org'



- Steve


On 2011-06-02 14:49:29, Nathan Binkert wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/726/
> ---
> 
> (Updated 2011-06-02 14:49:29)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> copyright: Add code for finding all copyright blocks and create a COPYING file
> 
> The end of the COPYING file was generated with:
> % python ./util/find_copyrights.py configs src system tests util
> 
> Update -C command line option to spit out COPYING file
> 
> [ Some notes on this change.  1) I could add the find_copyrights.py function 
> to the build to automatically generate the list.  Should we do that?  For 
> now, I've appended the list to the COPYING file.  We should also probably get 
> rid of the AUTHORS file.  What do you all think? (either that or we should 
> update it) ]
> 
> 
> Diffs
> -
> 
>   COPYING PRE-CREATION 
>   LICENSE aa00cee9abb1 
>   src/SConscript aa00cee9abb1 
>   src/python/m5/main.py aa00cee9abb1 
>   util/find_copyrights.py PRE-CREATION 
> 
> Diff: http://reviews.m5sim.org/r/726/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Nathan
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: scons: rename TraceFlags to DebugFlags

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/728/#review1297
---

Ship it!


- Steve


On 2011-06-02 14:49:43, Nathan Binkert wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/728/
> ---
> 
> (Updated 2011-06-02 14:49:43)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> scons: rename TraceFlags to DebugFlags
> 
> 
> Diffs
> -
> 
>   src/SConscript aa00cee9abb1 
>   src/arch/SConscript aa00cee9abb1 
>   src/arch/arm/SConscript aa00cee9abb1 
>   src/arch/mips/SConscript aa00cee9abb1 
>   src/arch/power/SConscript aa00cee9abb1 
>   src/arch/sparc/SConscript aa00cee9abb1 
>   src/arch/x86/SConscript aa00cee9abb1 
>   src/base/SConscript aa00cee9abb1 
>   src/base/vnc/SConscript aa00cee9abb1 
>   src/cpu/SConscript aa00cee9abb1 
>   src/cpu/inorder/SConscript aa00cee9abb1 
>   src/cpu/o3/SConscript aa00cee9abb1 
>   src/cpu/ozone/SConscript aa00cee9abb1 
>   src/cpu/pred/SConscript aa00cee9abb1 
>   src/cpu/simple/SConscript aa00cee9abb1 
>   src/cpu/testers/directedtest/SConscript aa00cee9abb1 
>   src/cpu/testers/memtest/SConscript aa00cee9abb1 
>   src/cpu/testers/networktest/SConscript aa00cee9abb1 
>   src/cpu/testers/rubytest/SConscript aa00cee9abb1 
>   src/dev/SConscript aa00cee9abb1 
>   src/dev/alpha/SConscript aa00cee9abb1 
>   src/dev/arm/SConscript aa00cee9abb1 
>   src/dev/mips/SConscript aa00cee9abb1 
>   src/dev/sparc/SConscript aa00cee9abb1 
>   src/dev/x86/SConscript aa00cee9abb1 
>   src/kern/SConscript aa00cee9abb1 
>   src/mem/SConscript aa00cee9abb1 
>   src/mem/cache/SConscript aa00cee9abb1 
>   src/mem/cache/tags/SConscript aa00cee9abb1 
>   src/sim/SConscript aa00cee9abb1 
> 
> Diff: http://reviews.m5sim.org/r/728/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Nathan
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: scons: rename some things from m5 to gem5

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/727/#review1296
---



src/SConscript
<http://reviews.m5sim.org/r/727/#comment1763>

Does it make sense to change the libname too?  Is there a reason not to?


- Steve


On 2011-06-02 14:51:25, Nathan Binkert wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/727/
> ---
> 
> (Updated 2011-06-02 14:51:25)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> scons: rename some things from m5 to gem5
> 
> The default generated binary is now gem5. instead of m5..
> The latter does still work but gem5. will be generated first and
> then m5. will be hard linked to it.
> 
> 
> Diffs
> -
> 
>   src/SConscript aa00cee9abb1 
> 
> Diff: http://reviews.m5sim.org/r/727/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Nathan
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: SConstruct: automatically update .hg/hgrc with style hooks

[Steve Reinhardt]

> On 2011-06-01 23:52:14, Gabe Black wrote:
> > SConstruct, line 204
> > <http://reviews.m5sim.org/r/668/diff/2/?file=12773#file12773line204>
> >
> > This is pretty reasonable, but it would be nice to mention that if it's 
> > not OK you can use Ctrl+C to get out of it. Usually Ctrl+C is a fairly 
> > drastic last resort, so people might not think to use it.

Yea, that crossed my mind... however, I still have the attitude that we're just 
informing the user about this change, not really asking permission, and the 
prompt is just there to make sure they see the notice. I'll try and make things 
a little clearer.


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/668/#review1294
-------


On 2011-06-01 21:27:23, Steve Reinhardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/668/
> ---
> 
> (Updated 2011-06-01 21:27:23)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> SConstruct: automatically update .hg/hgrc with style hooks
> 
> Seems easier than pestering people about it.
> Note also that path is now absolute, so you don't get errors
> when invoking hg from subdirectories.
> 
> 
> Diffs
> -
> 
>   SConstruct 3f49ed206f46 
> 
> Diff: http://reviews.m5sim.org/r/668/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in m5: SimObject: allow modules in subclass definitions

[Steve Reinhardt]

changeset aa00cee9abb1 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=aa00cee9abb1
description:
SimObject: allow modules in subclass definitions

In particular, this avoids crashing when you do
an import (like "import pdb") inside a SimObject
subclass definition.

diffstat:

 src/python/m5/SimObject.py |  5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diffs (22 lines):

diff -r 681497e0356b -r aa00cee9abb1 src/python/m5/SimObject.py
--- a/src/python/m5/SimObject.pyTue May 31 02:56:22 2011 -0400
+++ b/src/python/m5/SimObject.pyWed Jun 01 21:43:13 2011 -0700
@@ -29,7 +29,7 @@
 #  Nathan Binkert
 
 import sys
-from types import FunctionType, MethodType
+from types import FunctionType, MethodType, ModuleType
 
 try:
 import pydot
@@ -130,7 +130,8 @@
 
 def public_value(key, value):
 return key.startswith('_') or \
-   isinstance(value, (FunctionType, MethodType, classmethod, type))
+   isinstance(value, (FunctionType, MethodType, ModuleType,
+  classmethod, type))
 
 # The metaclass for SimObject.  This class controls how new classes
 # that derive from SimObject are instantiated, and provides inherited
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Debug Flags

[Steve Reinhardt]

OK, now that I'm at a keyboard: if we change the scons thing from
TraceFlag() to DebugFlag(), then there's one set of flags with one set
of names, it's all debug flags.  Some (currently most) of them are
only used to control DPRINTFs, but they could be used for other
things, and the same flag could in theory be used for both DPRINTFs
and other things.

All the --trace-* options solely control things related to DPRINTFs
and/or exec tracing (I believe), so that's consistent too.

It might not be the simplest scheme ever, but it sounds simpler than
having distinct debug flags and trace flags.

Steve

On Wed, Jun 1, 2011 at 8:15 PM, Steve Reinhardt  wrote:
> On my phone, so I'll be brief, but just fixing the scons to be consistent
> sounds good to me.
>
> On Jun 1, 2011 6:55 PM, "Gabriel Michael Black" 
> wrote:
>> So, I think part of the confusion is that there are two names now,
>> debug flags and trace flags, but they're different views of the same
>> mechanism (yes? no?) It seems like the --trace* options are like the
>> --debug* options, except their intended use is a subset of --debug*,
>> specifically DPRINTFs. What about returning the DPRINTF ones to
>> --trace-flags, etc., and introducing a separate parallel set of
>> options and namespace for the debug stuff? There's some macro or
>> something to check if trace flags are turned on, and that encourages
>> their use as debug flags (although I think that use is minimal in the
>> current code). We could introduce a new DEBUG_ON() macro (or a better
>> name) and optionally eliminate the trace oriented one or make it
>> internal to DPRINTFs only. I can think of some valid uses for keeping
>> it like blocks of DPRINTFs like Ali added recently, but it blurs the
>> line and could add to the confusion.
>>
>> By having two parallel systems, even though they're a bit redundant
>> where they overlap, I think it introduces a clear conceptual
>> separation between the two. Then it's clear what trace flags are for
>> and when to use them, and also what debug flags are for and when to
>> use them.
>>
>> We really have two different ideas budding off from each other
>> (controlling tracing and debug features), and by partially bundling
>> them together and partially distinguishing them that leads to
>> confusion. The mental model is different from the way you have to
>> control things, and trying to reconcile the two views makes the system
>> hard to reason about.
>>
>> Gabe
>>
>> Quoting nathan binkert :
>>
>>> Ok, there has been a lot of confusion about debug flags and trace
>>> flags. I changed the way the flags stuff worked from a compile
>>> perspective which required me to make changes throughout the tree, so
>>> I took the opportunity to rename the trace flags to debug flags. The
>>> idea behind the change was that the flags can be used for things other
>>> than tracing (I use them for breakpoints) and there is only one
>>> namespace, so I just renamed it to debug (people did review that
>>> change).
>>>
>>> So, I renamed --trace-flags to --debug-flags and --trace-flags-help to
>>> --debug-flags-help. --trace-start, --trace-file, and --trace-ignore
>>> stayed the same because those only affect the tracing portion of the
>>> debugging stuff. I never renamed the TraceFlags SCons option to
>>> DebugFlags.
>>>
>>> So, how do we clear up the confusion? Should I just fix the SCons
>>> thing and people will just learn? Should I change the name back?
>>> (There are a ton of places where this would change).
>>>
>>> Anyone care?
>>>
>>> Nate
>>> ___
>>> gem5-dev mailing list
>>> gem5-dev@m5sim.org
>>> http://m5sim.org/mailman/listinfo/gem5-dev
>>>
>>
>>
>> ___
>> gem5-dev mailing list
>> gem5-dev@m5sim.org
>> http://m5sim.org/mailman/listinfo/gem5-dev
>
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: SConstruct: automatically update .hg/hgrc with style hooks

[Steve Reinhardt]

Reviewboard didn't give me a chance to comment, but this is basically the
same as before except I now prompt for an input before modifying the
.hg/hgrc.  The truly paranoid can hit ctrl-c and get out of there.  I
thought it would be nice to get this in before the tutorial, since ideally
we'll have a bunch of newbies cloning new repositories.

Steve

On Wed, Jun 1, 2011 at 9:27 PM, Steve Reinhardt  wrote:

>This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/668/
>   Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and
> Nathan Binkert.
> By Steve Reinhardt.
>
> *Updated 2011-06-01 21:27:23.466278*
> Description
>
> SConstruct: automatically update .hg/hgrc with style hooks
>
> Seems easier than pestering people about it.
> Note also that path is now absolute, so you don't get errors
> when invoking hg from subdirectories.
>
>   Diffs (updated)
>
>- SConstruct (3f49ed206f46)
>
> View Diff <http://reviews.m5sim.org/r/668/diff/>
>
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: SConstruct: automatically update .hg/hgrc with style hooks

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/668/
---

(Updated 2011-06-01 21:27:23.466278)


Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

SConstruct: automatically update .hg/hgrc with style hooks

Seems easier than pestering people about it.
Note also that path is now absolute, so you don't get errors
when invoking hg from subdirectories.


Diffs (updated)
-

  SConstruct 3f49ed206f46 

Diff: http://reviews.m5sim.org/r/668/diff


Testing
---


Thanks,

Steve

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Debug Flags

[Steve Reinhardt]

On my phone, so I'll be brief, but just fixing the scons to be consistent
sounds good to me.
On Jun 1, 2011 6:55 PM, "Gabriel Michael Black" 
wrote:
> So, I think part of the confusion is that there are two names now,
> debug flags and trace flags, but they're different views of the same
> mechanism (yes? no?) It seems like the --trace* options are like the
> --debug* options, except their intended use is a subset of --debug*,
> specifically DPRINTFs. What about returning the DPRINTF ones to
> --trace-flags, etc., and introducing a separate parallel set of
> options and namespace for the debug stuff? There's some macro or
> something to check if trace flags are turned on, and that encourages
> their use as debug flags (although I think that use is minimal in the
> current code). We could introduce a new DEBUG_ON() macro (or a better
> name) and optionally eliminate the trace oriented one or make it
> internal to DPRINTFs only. I can think of some valid uses for keeping
> it like blocks of DPRINTFs like Ali added recently, but it blurs the
> line and could add to the confusion.
>
> By having two parallel systems, even though they're a bit redundant
> where they overlap, I think it introduces a clear conceptual
> separation between the two. Then it's clear what trace flags are for
> and when to use them, and also what debug flags are for and when to
> use them.
>
> We really have two different ideas budding off from each other
> (controlling tracing and debug features), and by partially bundling
> them together and partially distinguishing them that leads to
> confusion. The mental model is different from the way you have to
> control things, and trying to reconcile the two views makes the system
> hard to reason about.
>
> Gabe
>
> Quoting nathan binkert :
>
>> Ok, there has been a lot of confusion about debug flags and trace
>> flags. I changed the way the flags stuff worked from a compile
>> perspective which required me to make changes throughout the tree, so
>> I took the opportunity to rename the trace flags to debug flags. The
>> idea behind the change was that the flags can be used for things other
>> than tracing (I use them for breakpoints) and there is only one
>> namespace, so I just renamed it to debug (people did review that
>> change).
>>
>> So, I renamed --trace-flags to --debug-flags and --trace-flags-help to
>> --debug-flags-help. --trace-start, --trace-file, and --trace-ignore
>> stayed the same because those only affect the tracing portion of the
>> debugging stuff. I never renamed the TraceFlags SCons option to
>> DebugFlags.
>>
>> So, how do we clear up the confusion? Should I just fix the SCons
>> thing and people will just learn? Should I change the name back?
>> (There are a ton of places where this would change).
>>
>> Anyone care?
>>
>> Nate
>> ___
>> gem5-dev mailing list
>> gem5-dev@m5sim.org
>> http://m5sim.org/mailman/listinfo/gem5-dev
>>
>
>
> ___
> gem5-dev mailing list
> gem5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/gem5-dev
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] registerThreadContext

[Steve Reinhardt]

On Wed, May 25, 2011 at 10:57 AM, nathan binkert  wrote:
>
> I see a few options for solving this problem:
> 1) Separate out the contextId allocation from registerThreadContext so
> things like DMA controllers and memtesters can get allocated a
> contextID.
> 2) Create a base class for ThreadContext that is far simpler than the
> current thread context and use that when registering.
> 3) Figure out contextID not by registration, but instead by doing a
> traversal of the memory system.  This would require that we have some
> sort of indication differentiating memory objects that can generate
> requests and thus require a contextID and memory objects that can't
> (caches, dram, pio devices, etc.).  We add a constructor parameter to
> the MemObject base class.
> 4) Add a separate registration function for non Thread Contexts.

While #3 sounds nice, it's a relatively big change, and would have to
be done in a way that works both for the m5 classic memory system and
for Ruby, and ideally is robust and predictable in assigning
contextIDs in the face of minor configuration changes.  My guess is
that it will end up being harder than it sounds... maybe Korey can
speak up if his experience indicates otherwise.

The others seem more reasonable, but possibly still overkill.

The idea I had, that's kind of a hack but combines #2 and #4 in a
degenerate sort of way, would be to allow non-ThreadContext objects to
call registerThreadContext, but they would pass in NULL for the
ThreadContext pointer.  This would allow something like a DMA device
or the memtester to reserve or allocate a context ID without being a
ThreadContext, but without creating an additional base class.  The
only down side I see is that error tracking would be harder; if two
non-ThreadContexts tried to claim the same ID, the System would not
have a pointer to be able to identify the first one.  Basically that's
the only case I can see where it would be useful for the System object
to hold on to some kind of pointer for something that's not an actual
ThreadContext.  Of course, the current ThreadContext pointers don't
have any real debug info either, so even now you just get "Cannot have
two CPUs with the same id (%d)\n" when you try to reuse a context ID,
so apparently it hasn't been a big problem.

If we cared about providing good error messages, we could extend
registerThreadContext to take both a SimObject* and a ThreadContext*
and track both of these, but make the latter optional.  This would
provide better error messages for everyone (though again, this hasn't
been a problem that I've seen).  registerThreadContext is only called
from one place (BaseCPU::registerThreadContexts()) so it wouldn't be
hard to change that.

Just some ideas...

Steve
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: ISA parser: Simplify operand type handling.

[Steve Reinhardt]

On Mon, May 30, 2011 at 1:33 PM, Gabe Black  wrote:
> On 05/30/11 09:47, Steve Reinhardt wrote:
>>
>> Anyway, it seems very odd to have to say "(int8_t)Mem.ub" when we already 
>> have a ".sb" operand type defined as "int8_t".  It seems like a weird hidden 
>> restriction to say that there are operand types you can declare but can't 
>> use on memory, and that you're pushing a somewhat arbitrary internal 
>> implementation decision up to the level of language visibility, which is 
>> going the wrong direction.  I'm not sure what the right solution is, but 
>> even if it's the brute force one of creating a bunch more read/write 
>> function templates in the CPU implementations, I think that's preferable.
> [...]
>
> The unsigned thing is sort of a weird gotcha. I'd like to avoid adding a
> lot of bulk to the CPU models since they're already fairly chunky and it
> makes them harder to reason about looking through the code, but it would
> be great to straighten this out. One possibility might be the technique
> I used with the endianness changing functions in src/sim/byteswap.hh.
> Things are built up in layers there:
> [...]

I think some kind of additional set of template instantiations should
do it.  I just noticed that we already have:

template<>
Fault
AtomicSimpleCPU::read(Addr addr, int32_t &data, unsigned flags)
{
return read(addr, (uint32_t&)data, flags);
}

template<>
Fault
AtomicSimpleCPU::write(int32_t data, Addr addr, unsigned flags, uint64_t *res)
{
return write((uint32_t)data, addr, flags, res);
}

.. and similar for TimingSimpleCPU; do we just need to extend these to
int8_t and int16_t, and maybe add similar sets in
base_dynamic_inst.hh?

Steve
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: ISA parser: Allow defining operand types with a ctype directly.

[Steve Reinhardt]

> On 2011-05-28 22:04:00, Steve Reinhardt wrote:
> > This looks fine to me, but I'm confused... don't you delete this code 
> > completely two patches from now?  Why bother changing it if you're going to 
> > get rid of it?
> 
> Gabe Black wrote:
> Because this way both are available, and the ISAs can be converted one at 
> a time and everything will work between every patch. Otherwise I'd have to 
> break everything that wasn't (or was) updated, or do everything in one big 
> change that does too much at once. Once all the ISAs are consistently on the 
> new system, the old system isn't needed anymore and is deleted in that later 
> patch.
> 
> Steve Reinhardt wrote:
> OK, I see.  Overall this seems like overkill; I can see how this code was 
> useful while you were developing, but for committing, one complete patch that 
> gets rid of the old way of doing things and fixes all the ISAs to use the new 
> way would be preferable to me.  I think it would be less confusing because 
> all the related changes would be right there in one place.  I don't have a 
> problem with large changesets (in terms of lines of code or files touched), 
> just ones that are not conceptually unified, and spreading this change across 
> multiple csets goes against "conceptually unified" in the opposite direction, 
> IMO.
> 
> Gabe Black wrote:
> That makes sense, but then I think when changes get to be too big, they 
> get to be too hard to understand all at once. By keeping each part relatively 
> small it makes things easier to digest later. My enormous PC state change is 
> an example of the opposite extreme, and it was probably a lot of work to get 
> through and review and overwhelming to look at later in the history. I'd like 
> to avoid that if possible.
> 
> Gabe Black wrote:
> Then again, the changes after this one don't add much complexity to this 
> one since they're pretty simple. Merging them is reasonable, and I'll do that 
> once we're ready to put these in.

Yea, that's what I was thinking... while patches can get pretty big, if you 
just combine the ISA-specific operand_type fixes (like 
http://reviews.m5sim.org/r/657, but basically doing the same thing to all the 
ISAs) with the one that permanently changes how the operand_type block is 
processed (http://reviews.m5sim.org/r/658, which basically makes this one 
irrelevant), then it's still not that big.  I'm definitely not suggesting that 
you fold in http://reviews.m5sim.org/r/655, just that you combine this one 
(656) with 657 (& siblings) and 658.


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/656/#review1277
---


On 2011-04-25 03:04:12, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/656/
> ---
> 
> (Updated 2011-04-25 03:04:12)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> ISA parser: Allow defining operand types with a ctype directly.
> 
> 
> Diffs
> -
> 
>   src/arch/isa_parser.py de679a068dd8 
> 
> Diff: http://reviews.m5sim.org/r/656/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabe
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: ISA parser: Simplify operand type handling.

[Steve Reinhardt]

> On 2011-05-28 21:58:27, Steve Reinhardt wrote:
> > The description is a little general; can you be more specific about what 
> > this change is doing?  I see that you're getting rid of the size from the 
> > memory access operands and encoding that in the ctype instead, which seems 
> > fine.  However it seems like you've gotten rid of a lot of the signed vs. 
> > unsigned code, and made everything look unsigned, and I don't see how that 
> > still works.
> 
> Gabe Black wrote:
> The reason I changed them to unsigned is that all the read/write 
> functions have definitions generated for unsigned int operands but not signed 
> ones. They were being forced to unsigned when the call was being made. All of 
> the regressions passed with these changes and at the time I was convinced why 
> this worked, but it's been about a month since then and I don't really 
> remember the specifics. In SPARC and ARM, the Mem variables are being cast to 
> an appropriate type in the assignment, and in Alpha there were apparently no 
> signed Mem types. I'm guessing MIPS and Power don't have as extensive 
> regressions so problems may have slipped through due to those instructions 
> not being used or being used in a way that didn't expose a difference in 
> behavior. The simple fix is to add casts to those too in the few places where 
> it makes a difference, although there may be a reason that I'm not able to 
> remember that it works as is.
> 
> Generally speaking, signed vs. unsigned, size, and float vs. int 
> information only makes sense if you're building up a few preselected types as 
> the code is currently doing. By deferring more to the C++ type system, you 
> can use whatever type you want and it should just work.

Well, the fact that the old regressions didn't notice doesn't fill me with 
confidence, but at least it's more plausible that the updated version works.  I 
guess your old version may have worked in the MIPS case because the LHS is 
signed, e.g., lh is "Rt.sw = Mem.uw" while lhu is "Rt.uw = Mem.uw", but the LHS 
is not signed in the Power case, and even in MIPS the size on the LHS for lb is 
wrong (it's also Rt.sw and not Rt.sb), which may or may not matter.

Anyway, it seems very odd to have to say "(int8_t)Mem.ub" when we already have 
a ".sb" operand type defined as "int8_t".  It seems like a weird hidden 
restriction to say that there are operand types you can declare but can't use 
on memory, and that you're pushing a somewhat arbitrary internal implementation 
decision up to the level of language visibility, which is going the wrong 
direction.  I'm not sure what the right solution is, but even if it's the brute 
force one of creating a bunch more read/write function templates in the CPU 
implementations, I think that's preferable.

I must say that other than this one inconsistency, I like this change, and I'm 
left scratching my head about why I made the original version so complicated in 
the first place.


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/655/#review1276
---


On 2011-05-30 00:18:31, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/655/
> ---
> 
> (Updated 2011-05-30 00:18:31)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> ISA parser: Simplify operand type handling.
> 
> This change simplifies the code surrounding operand type handling and makes it
> depend only on the ctype that goes with each operand type. Future changes will
> allow defining operand types by their ctypes directly, convert the ISAs over
> to that style of definition, and then remove support for the old style. These
> changes are to make it easier to use non-builtin types like classes or
> structures as the type for operands.
> 
> 
> Diffs
> -
> 
>   src/arch/alpha/isa/mem.isa 03cfd2ecf6bb 
>   src/arch/arm/isa/insts/ldr.isa 03cfd2ecf6bb 
>   src/arch/arm/isa/insts/mem.isa 03cfd2ecf6bb 
>   src/arch/arm/isa/insts/str.isa 03cfd2ecf6bb 
>   src/arch/arm/isa/templates/mem.isa 03cfd2ecf6bb 
>   src/arch/isa_parser.py 03cfd2ecf6bb 
>   src/arch/mips/isa/decoder.isa 03cfd2ecf6bb 
>   src/arch/mips/isa/formats/mem.isa 03cfd2ecf6bb 
>   src/arch/power/isa/decoder.isa 03cfd2ecf6bb 
>   src/arch/power/isa/formats/mem.

Re: [gem5-dev] Review Request: ISA parser: Allow defining operand types with a ctype directly.

[Steve Reinhardt]

> On 2011-05-28 22:04:00, Steve Reinhardt wrote:
> > This looks fine to me, but I'm confused... don't you delete this code 
> > completely two patches from now?  Why bother changing it if you're going to 
> > get rid of it?
> 
> Gabe Black wrote:
> Because this way both are available, and the ISAs can be converted one at 
> a time and everything will work between every patch. Otherwise I'd have to 
> break everything that wasn't (or was) updated, or do everything in one big 
> change that does too much at once. Once all the ISAs are consistently on the 
> new system, the old system isn't needed anymore and is deleted in that later 
> patch.

OK, I see.  Overall this seems like overkill; I can see how this code was 
useful while you were developing, but for committing, one complete patch that 
gets rid of the old way of doing things and fixes all the ISAs to use the new 
way would be preferable to me.  I think it would be less confusing because all 
the related changes would be right there in one place.  I don't have a problem 
with large changesets (in terms of lines of code or files touched), just ones 
that are not conceptually unified, and spreading this change across multiple 
csets goes against "conceptually unified" in the opposite direction, IMO.


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/656/#review1277
---


On 2011-04-25 03:04:12, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/656/
> ---
> 
> (Updated 2011-04-25 03:04:12)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> ISA parser: Allow defining operand types with a ctype directly.
> 
> 
> Diffs
> -
> 
>   src/arch/isa_parser.py de679a068dd8 
> 
> Diff: http://reviews.m5sim.org/r/656/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabe
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: ISA parser: Stop supporting the old style operand types.

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/658/#review1279
---


See previous reviews... this looks fine if all the ISAs are converted to use 
it.  Once it's committed, the relevant documentation should also be updated 
(e.g., http://gem5.org/Code_parsing#Operand_type_qualifiers).

- Steve


On 2011-04-25 03:04:33, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/658/
> ---
> 
> (Updated 2011-04-25 03:04:33)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> ISA parser: Stop supporting the old style operand types.
> 
> 
> Diffs
> -
> 
>   src/arch/isa_parser.py de679a068dd8 
> 
> Diff: http://reviews.m5sim.org/r/658/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabe
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: X86: Convert operand types to the new style.

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/657/#review1278
---


Looks fine, but shouldn't you be doing all the ISAs if you're going to get rid 
of the old style?

- Steve


On 2011-04-25 03:04:20, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/657/
> ---
> 
> (Updated 2011-04-25 03:04:20)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> X86: Convert operand types to the new style.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/isa/operands.isa de679a068dd8 
> 
> Diff: http://reviews.m5sim.org/r/657/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabe
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: ISA parser: Allow defining operand types with a ctype directly.

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/656/#review1277
---


This looks fine to me, but I'm confused... don't you delete this code 
completely two patches from now?  Why bother changing it if you're going to get 
rid of it?

- Steve


On 2011-04-25 03:04:12, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/656/
> ---
> 
> (Updated 2011-04-25 03:04:12)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> ISA parser: Allow defining operand types with a ctype directly.
> 
> 
> Diffs
> -
> 
>   src/arch/isa_parser.py de679a068dd8 
> 
> Diff: http://reviews.m5sim.org/r/656/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabe
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: ISA parser: Simplify operand type handling.

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/655/#review1276
---


The description is a little general; can you be more specific about what this 
change is doing?  I see that you're getting rid of the size from the memory 
access operands and encoding that in the ctype instead, which seems fine.  
However it seems like you've gotten rid of a lot of the signed vs. unsigned 
code, and made everything look unsigned, and I don't see how that still works.


src/arch/mips/isa/decoder.isa
<http://reviews.m5sim.org/r/655/#comment1753>

This confuses me... how is it that lb & lbu (and lh & lhu) have identical 
definitions now?  What is it that makes lb and lh signed?


- Steve


On 2011-04-25 03:03:53, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/655/
> ---
> 
> (Updated 2011-04-25 03:03:53)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> ISA parser: Simplify operand type handling.
> 
> This change simplifies the code surrounding operand type handling and makes it
> depend only on the ctype that goes with each operand type. Future changes will
> allow defining operand types by their ctypes directly, convert the ISAs over
> to that style of definition, and then remove support for the old style. These
> changes are to make it easier to use non-builtin types like classes or
> structures as the type for operands.
> 
> 
> Diffs
> -
> 
>   src/arch/alpha/isa/mem.isa de679a068dd8 
>   src/arch/arm/isa/insts/ldr.isa de679a068dd8 
>   src/arch/arm/isa/insts/mem.isa de679a068dd8 
>   src/arch/arm/isa/insts/str.isa de679a068dd8 
>   src/arch/arm/isa/templates/mem.isa de679a068dd8 
>   src/arch/isa_parser.py de679a068dd8 
>   src/arch/mips/isa/decoder.isa de679a068dd8 
>   src/arch/mips/isa/formats/mem.isa de679a068dd8 
>   src/arch/power/isa/decoder.isa de679a068dd8 
>   src/arch/power/isa/formats/mem.isa de679a068dd8 
>   src/arch/sparc/isa/decoder.isa de679a068dd8 
>   src/arch/sparc/isa/formats/mem/swap.isa de679a068dd8 
>   src/arch/sparc/isa/formats/mem/util.isa de679a068dd8 
> 
> Diff: http://reviews.m5sim.org/r/655/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabe
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: Misc: Remove the URL from warnings, fatals, panics, etc.

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/719/#review1275
---

Ship it!


- Steve


On 2011-05-25 09:25:20, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/719/
> ---
> 
> (Updated 2011-05-25 09:25:20)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> Misc: Remove the URL from warnings, fatals, panics, etc.
> 
> 
> Diffs
> -
> 
>   src/base/misc.cc dda2a88eb7c4 
>   src/python/m5/util/__init__.py dda2a88eb7c4 
> 
> Diff: http://reviews.m5sim.org/r/719/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabe
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: Config: Add support for a Self.all proxy object

[Steve Reinhardt]

> On 2011-05-28 17:22:15, Steve Reinhardt wrote:
> > src/sim/System.py, line 47
> > <http://reviews.m5sim.org/r/720/diff/1/?file=12676#file12676line47>
> >
> > It seems odd that Parent.any here will generate an error if there are 
> > multiple matches, but Self.all only is necessary if there are multiple 
> > matches.  I think the reason it's not a problem is that this Parent.any 
> > proxy on physmem is never used.  Should we get rid of it?
> 
> Ali Saidi wrote:
> You mean we set it to something and we should make it have no default? 
> Another option would be to change the way this works a bit. Just use the 
> memories above.

Getting rid of the Parent.any default would be one step; merging physmem and 
memories would be even better since they seem redundant, correct?  What about 
keeping the physmem name and replacing it with the definition you have for 
memories?


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/720/#review1272
---


On 2011-05-26 19:17:18, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/720/
> ---
> 
> (Updated 2011-05-26 19:17:18)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> Config: Add support for a Self.all proxy object
> 
> 
> Diffs
> -
> 
>   src/python/m5/SimObject.py 3f37cc5d25bc 
>   src/python/m5/params.py 3f37cc5d25bc 
>   src/python/m5/proxy.py 3f37cc5d25bc 
>   src/sim/System.py 3f37cc5d25bc 
> 
> Diff: http://reviews.m5sim.org/r/720/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ali
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: Config: Add support for a Self.all proxy object

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/720/#review1272
---



src/sim/System.py
<http://reviews.m5sim.org/r/720/#comment1750>

It seems odd that Parent.any here will generate an error if there are 
multiple matches, but Self.all only is necessary if there are multiple matches. 
 I think the reason it's not a problem is that this Parent.any proxy on physmem 
is never used.  Should we get rid of it?


- Steve


On 2011-05-26 19:17:18, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/720/
> ---
> 
> (Updated 2011-05-26 19:17:18)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> Config: Add support for a Self.all proxy object
> 
> 
> Diffs
> -
> 
>   src/python/m5/SimObject.py 3f37cc5d25bc 
>   src/python/m5/params.py 3f37cc5d25bc 
>   src/python/m5/proxy.py 3f37cc5d25bc 
>   src/sim/System.py 3f37cc5d25bc 
> 
> Diff: http://reviews.m5sim.org/r/720/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ali
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: Config: Add support for a Self.all proxy object

[Steve Reinhardt]

> On 2011-05-28 09:20:00, Ali Saidi wrote:
> > src/python/m5/proxy.py, line 187
> > <http://reviews.m5sim.org/r/720/diff/1/?file=12675#file12675line187>
> >
> > Parent.all would find every object above you in the hierarchy that 
> > matched, although I've never tried it. I only use self.all..
> > 
> > I can add a description for those two. Will you add one for the rest of 
> > the proxy objets? :)

After looking at BaseProxy.unproxy(), I'm pretty sure Parent.all will not work, 
since the base algorithm quits as soon as it sees a find() method return True.  
I don't think that's a huge problem, but it would be good to find a way to 
return an error on Parent.all instead of letting people find out the hard way 
that it doesn't work.


> On 2011-05-28 09:20:00, Ali Saidi wrote:
> > src/python/m5/params.py, line 187
> > <http://reviews.m5sim.org/r/720/diff/1/?file=12674#file12674line187>
> >
> > The all proxy object is going to return an array which in then going to 
> > get turned into [[all,objects,that,match]]. we don't want this.

I understand why this code is necessary, but that doesn't mean I like it ;-).  
It seems like an arbitrary hack that could possibly turn around and bite us at 
some point.

Basically the current code assumes that a proxied VectorParam is a vector of 
scalar proxies, not a single proxy object that's going to return a vector of 
things.  This code assumes that if the first element of the unproxied vector is 
a vector, then you really want that and not the original vector.  Seems a 
little broad to me.  How about something like:

if len(self) == 1 and isinstance(self[0], AllProxy):
return self[0].unproxy(base)
else:
return [v.unproxy(base) for v in self]


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/720/#review1269
---


On 2011-05-26 19:17:18, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/720/
> -------
> 
> (Updated 2011-05-26 19:17:18)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> Config: Add support for a Self.all proxy object
> 
> 
> Diffs
> -
> 
>   src/python/m5/SimObject.py 3f37cc5d25bc 
>   src/python/m5/params.py 3f37cc5d25bc 
>   src/python/m5/proxy.py 3f37cc5d25bc 
>   src/sim/System.py 3f37cc5d25bc 
> 
> Diff: http://reviews.m5sim.org/r/720/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ali
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: Config: Add support for a Self.all proxy object

[Steve Reinhardt]

> On 2011-05-26 23:29:05, Gabe Black wrote:
> > What does Self.all do? What is it for? You have one use in the change so I 
> > have a basic idea, but it would be helpful to know the specifics.
> 
> Ali Saidi wrote:
> It's similar to Parent.any in that it traverses the object hierarchy and 
> finds all  (as opposed to one) objects that are of a certain type.  As you 
> see from the memories example, the system object can have a pointer to all 
> PhysicalMemory objects that belong to that system. The reason to add 
> something like this, is to have a generic way to prevent speculation from 
> touching I/O. It's possible for a speculative instruction fetch to sneak out 
> of the CPU if the wrong 10 things all happen an once and with this another 
> change can prevent that (will post soon). It's not as simple as just not 
> fetching from non-cacheable memory, since most architectures start their 
> processors in some kind of caches disabled, tlb should only issue 
> non-cachable transaction state.

So if the real machine is using speculative execution in this cache-disable 
mode, how does it avoid touching I/O locations?


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/720/#review1262
---


On 2011-05-26 19:17:18, Ali Saidi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/720/
> ---
> 
> (Updated 2011-05-26 19:17:18)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> Config: Add support for a Self.all proxy object
> 
> 
> Diffs
> -
> 
>   src/python/m5/SimObject.py 3f37cc5d25bc 
>   src/python/m5/params.py 3f37cc5d25bc 
>   src/python/m5/proxy.py 3f37cc5d25bc 
>   src/sim/System.py 3f37cc5d25bc 
> 
> Diff: http://reviews.m5sim.org/r/720/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ali
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: O3: Create a pipeline activity viewer for the O3 CPU model.

[Steve Reinhardt]

Cool!  I haven't looked at the code yet, but the output looks nice.

Steve

On Thu, May 26, 2011 at 8:19 PM, Ali Saidi  wrote:
> Anyone that is curious what this looks like... lets try the picture again...
>
>
>
>
> Ali
>
> On May 26, 2011, at 9:19 PM, Ali Saidi wrote:
>
>>
>> ---
>> This is an automatically generated e-mail. To reply, visit:
>> http://reviews.m5sim.org/r/721/
>> ---
>>
>> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
>> Nathan Binkert.
>>
>>
>> Summary
>> ---
>>
>> O3: Create a pipeline activity viewer for the O3 CPU model.
>>
>> Implement a pipeline activity viewer as a python script (util/o3-pipeview.py)
>> and modified O3 code base to support an extra trace flag (O3PipeView) for
>> generating traces to be used as inputs by the tool.
>>
>>
>> Diffs
>> -
>>
>> src/cpu/SConscript 3f37cc5d25bc
>> src/cpu/o3/commit_impl.hh 3f37cc5d25bc
>> src/cpu/o3/decode_impl.hh 3f37cc5d25bc
>> src/cpu/o3/dyn_inst.hh 3f37cc5d25bc
>> src/cpu/o3/fetch_impl.hh 3f37cc5d25bc
>> src/cpu/o3/iew_impl.hh 3f37cc5d25bc
>> src/cpu/o3/inst_queue_impl.hh 3f37cc5d25bc
>> src/cpu/o3/rename_impl.hh 3f37cc5d25bc
>> util/o3-pipeview.py PRE-CREATION
>>
>> Diff: http://reviews.m5sim.org/r/721/diff
>>
>>
>> Testing
>> ---
>>
>>
>> Thanks,
>>
>> Ali
>>
>> ___
>> gem5-dev mailing list
>> gem5-dev@m5sim.org
>> http://m5sim.org/mailman/listinfo/gem5-dev
>>
>
>
> ___
> gem5-dev mailing list
> gem5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/gem5-dev
>
>
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: Name: Replace M5 with gem5 in a few places it's printed on startup.

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/713/#review1256
---

Ship it!



src/python/m5/main.py
<http://reviews.m5sim.org/r/713/#comment1739>

This should be "gem5 options" here


- Steve


On 2011-05-24 00:42:10, Gabe Black wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/713/
> ---
> 
> (Updated 2011-05-24 00:42:10)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> Name: Replace M5 with gem5 in a few places it's printed on startup.
> 
> 
> Diffs
> -
> 
>   src/python/m5/main.py 76095b05f4da 
> 
> Diff: http://reviews.m5sim.org/r/713/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gabe
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in m5: sim: style fixes in sim/process.hh

[Steve Reinhardt]

changeset 76095b05f4da in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=76095b05f4da
description:
sim: style fixes in sim/process.hh

diffstat:

 src/sim/process.hh |  46 +++---
 1 files changed, 19 insertions(+), 27 deletions(-)

diffs (104 lines):

diff -r aa7a67647c7b -r 76095b05f4da src/sim/process.hh
--- a/src/sim/process.hhMon May 23 14:29:23 2011 -0700
+++ b/src/sim/process.hhMon May 23 14:29:23 2011 -0700
@@ -126,7 +126,7 @@
 
   protected:
 // constructor
-Process(ProcessParams * params);
+Process(ProcessParams *params);
 
 virtual void initState();
 
@@ -144,30 +144,21 @@
 class FdMap
 {
   public:
-int fd;
-std::string filename;
-int mode;
-int flags;
-bool isPipe;
-int readPipeSource;
-uint64_t fileOffset;
+int fd;
+std::string filename;
+int mode;
+int flags;
+bool isPipe;
+int readPipeSource;
+uint64_t fileOffset;
 
-
-FdMap()
-{
-fd = -1;
-filename = "NULL";
-mode = 0;
-flags = 0;
-isPipe = false;
-readPipeSource = 0;
-fileOffset = 0;
-
-}
+FdMap()
+: fd(-1), filename("NULL"), mode(0), flags(0),
+  isPipe(false), readPipeSource(0), fileOffset(0)
+{ }
 
 void serialize(std::ostream &os);
 void unserialize(Checkpoint *cp, const std::string §ion);
-
 };
 
   private:
@@ -192,13 +183,14 @@
 }
 
 // Find a free context to use
-ThreadContext * findFreeContext();
+ThreadContext *findFreeContext();
 
 // map simulator fd sim_fd to target fd tgt_fd
 void dup_fd(int sim_fd, int tgt_fd);
 
 // generate new target fd for sim_fd
-int alloc_fd(int sim_fd, std::string filename, int flags, int mode, bool 
pipe);
+int alloc_fd(int sim_fd, std::string filename, int flags, int mode,
+ bool pipe);
 
 // free target fd (e.g., after close)
 void free_fd(int tgt_fd);
@@ -207,7 +199,7 @@
 int sim_fd(int tgt_fd);
 
 // look up simulator fd_map object for a given target fd
-FdMap * sim_fd_obj(int tgt_fd);
+FdMap *sim_fd_obj(int tgt_fd);
 
 // fix all offsets for currently open files and save them
 void fix_file_offsets();
@@ -240,7 +232,7 @@
 std::vector envp;
 std::string cwd;
 
-LiveProcess(LiveProcessParams * params, ObjectFile *objFile);
+LiveProcess(LiveProcessParams *params, ObjectFile *objFile);
 
 // Id of the owner of the process
 uint64_t __uid;
@@ -316,12 +308,12 @@
 virtual void setSyscallReturn(ThreadContext *tc,
 SyscallReturn return_value) = 0;
 
-virtual SyscallDesc* getDesc(int callnum) = 0;
+virtual SyscallDesc *getDesc(int callnum) = 0;
 
 // this function is used to create the LiveProcess object, since
 // we can't tell which subclass of LiveProcess to use until we
 // open and look at the object file.
-static LiveProcess *create(LiveProcessParams * params);
+static LiveProcess *create(LiveProcessParams *params);
 };
 
 
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in m5: syscall emul: fix Power Linux mmap constant, pl...

[Steve Reinhardt]

changeset aa7a67647c7b in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=aa7a67647c7b
description:
syscall emul: fix Power Linux mmap constant, plus other cleanup

We were getting a spurious warning in the regressions that turned
out to be due to having the wrong value for TGT_MAP_ANONYMOUS for
Power Linux, but in the process of tracking it down I ended up
doing some cleanup of the mmap handling in general.

diffstat:

 src/arch/power/linux/linux.hh |   2 +-
 src/sim/process.cc|   6 +++---
 src/sim/syscall_emul.hh   |  31 ++-
 3 files changed, 22 insertions(+), 17 deletions(-)

diffs (92 lines):

diff -r fd20dcf1a9aa -r aa7a67647c7b src/arch/power/linux/linux.hh
--- a/src/arch/power/linux/linux.hh Mon May 23 14:29:23 2011 -0700
+++ b/src/arch/power/linux/linux.hh Mon May 23 14:29:23 2011 -0700
@@ -126,7 +126,7 @@
 //@}
 
 /// For mmap().
-static const unsigned TGT_MAP_ANONYMOUS = 0x800;
+static const unsigned TGT_MAP_ANONYMOUS = 0x20;
 
 //@{
 /// ioctl() command codes.
diff -r fd20dcf1a9aa -r aa7a67647c7b src/sim/process.cc
--- a/src/sim/process.ccMon May 23 14:29:23 2011 -0700
+++ b/src/sim/process.ccMon May 23 14:29:23 2011 -0700
@@ -313,7 +313,7 @@
 int
 Process::sim_fd(int tgt_fd)
 {
-if (tgt_fd > MAX_FD)
+if (tgt_fd < 0 || tgt_fd > MAX_FD)
 return -1;
 
 return fd_map[tgt_fd].fd;
@@ -322,8 +322,8 @@
 Process::FdMap *
 Process::sim_fd_obj(int tgt_fd)
 {
-if (tgt_fd > MAX_FD)
-panic("sim_fd_obj called in fd out of range.");
+if (tgt_fd < 0 || tgt_fd > MAX_FD)
+return NULL;
 
 return &fd_map[tgt_fd];
 }
diff -r fd20dcf1a9aa -r aa7a67647c7b src/sim/syscall_emul.hh
--- a/src/sim/syscall_emul.hh   Mon May 23 14:29:23 2011 -0700
+++ b/src/sim/syscall_emul.hh   Mon May 23 14:29:23 2011 -0700
@@ -989,13 +989,8 @@
 /// We don't really handle mmap().  If the target is mmaping an
 /// anonymous region or /dev/zero, we can get away with doing basically
 /// nothing (since memory is initialized to zero and the simulator
-/// doesn't really check addresses anyway).  Always print a warning,
-/// since this could be seriously broken if we're not mapping
-/// /dev/zero.
-//
-/// Someday we should explicitly check for /dev/zero in open, flag the
-/// file descriptor, and fail (or implement!) a non-anonymous mmap to
-/// anything else.
+/// doesn't really check addresses anyway).
+///
 template 
 SyscallReturn
 mmapFunc(SyscallDesc *desc, int num, LiveProcess *p, ThreadContext *tc)
@@ -1005,9 +1000,24 @@
 uint64_t length = p->getSyscallArg(tc, index);
 index++; // int prot = p->getSyscallArg(tc, index);
 int flags = p->getSyscallArg(tc, index);
-int fd = p->sim_fd(p->getSyscallArg(tc, index));
+int tgt_fd = p->getSyscallArg(tc, index);
 // int offset = p->getSyscallArg(tc, index);
 
+if (!(flags & OS::TGT_MAP_ANONYMOUS)) {
+Process::FdMap *fd_map = p->sim_fd_obj(tgt_fd);
+if (!fd_map || fd_map->fd < 0) {
+warn("mmap failing: target fd %d is not valid\n", tgt_fd);
+return -EBADF;
+}
+
+if (fd_map->filename != "/dev/zero") {
+// This is very likely broken, but leave a warning here
+// (rather than panic) in case /dev/zero is known by
+// another name on some platform
+warn("allowing mmap of file %s; mmap not supported on files"
+ " other than /dev/zero\n", fd_map->filename);
+}
+}
 
 if ((start  % TheISA::VMPageSize) != 0 ||
 (length % TheISA::VMPageSize) != 0) {
@@ -1032,11 +1042,6 @@
 }
 p->pTable->allocate(start, length);
 
-if (!(flags & OS::TGT_MAP_ANONYMOUS)) {
-warn("allowing mmap of file @ fd %d. "
- "This will break if not /dev/zero.", fd);
-}
-
 return start;
 }
 
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] changeset in m5: config: revamp x86 config to avoid appending to...

[Steve Reinhardt]

changeset fd20dcf1a9aa in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=fd20dcf1a9aa
description:
config: revamp x86 config to avoid appending to SimObjectVectors
A significant contributor to the need for adoptOrphanParams()
is the practice of appending to SimObjectVectors which have
already been assigned as children.  This practice sidesteps the
assignment operation for those appended SimObjects, which is
where parent/child relationships are typically established.

This patch reworks the config scripts that use append() on
SimObjectVectors, which all happen to be in the x86 system
configuration.  At some point in the future, I hope to make
SimObjectVectors immutable (by deriving from tuple rather than
list), at which time this patch will be necessary for correct
operation.  For now, it just avoids some of the warning
messages that get printed in adoptOrphanParams().

diffstat:

 configs/common/FSConfig.py |  40 +---
 src/arch/x86/bios/E820.py  |   2 +-
 src/dev/x86/SouthBridge.py |  29 ++---
 3 files changed, 32 insertions(+), 39 deletions(-)

diffs (168 lines):

diff -r 19949c6de823 -r fd20dcf1a9aa configs/common/FSConfig.py
--- a/configs/common/FSConfig.pyMon May 23 14:29:23 2011 -0700
+++ b/configs/common/FSConfig.pyMon May 23 14:29:23 2011 -0700
@@ -306,7 +306,7 @@
 
 def x86IOAddress(port):
 IO_address_space_base = 0x8000
-return IO_address_space_base + port;
+return IO_address_space_base + port
 
 def connectX86ClassicSystem(x86_sys):
 x86_sys.membus = MemBus(bus_id=1)
@@ -375,27 +375,29 @@
 self.smbios_table.structures = structures
 
 # Set up the Intel MP table
+base_entries = []
+ext_entries = []
 for i in xrange(numCPUs):
 bp = X86IntelMPProcessor(
 local_apic_id = i,
 local_apic_version = 0x14,
 enable = True,
 bootstrap = (i == 0))
-self.intel_mp_table.add_entry(bp)
+base_entries.append(bp)
 io_apic = X86IntelMPIOAPIC(
 id = numCPUs,
 version = 0x11,
 enable = True,
 address = 0xfec0)
 self.pc.south_bridge.io_apic.apic_id = io_apic.id
-self.intel_mp_table.add_entry(io_apic)
+base_entries.append(io_apic)
 isa_bus = X86IntelMPBus(bus_id = 0, bus_type='ISA')
-self.intel_mp_table.add_entry(isa_bus)
+base_entries.append(isa_bus)
 pci_bus = X86IntelMPBus(bus_id = 1, bus_type='PCI')
-self.intel_mp_table.add_entry(pci_bus)
+base_entries.append(pci_bus)
 connect_busses = X86IntelMPBusHierarchy(bus_id=0,
 subtractive_decode=True, parent_bus=1)
-self.intel_mp_table.add_entry(connect_busses)
+ext_entries.append(connect_busses)
 pci_dev4_inta = X86IntelMPIOIntAssignment(
 interrupt_type = 'INT',
 polarity = 'ConformPolarity',
@@ -404,7 +406,7 @@
 source_bus_irq = 0 + (4 << 2),
 dest_io_apic_id = io_apic.id,
 dest_io_apic_intin = 16)
-self.intel_mp_table.add_entry(pci_dev4_inta);
+base_entries.append(pci_dev4_inta)
 def assignISAInt(irq, apicPin):
 assign_8259_to_apic = X86IntelMPIOIntAssignment(
 interrupt_type = 'ExtInt',
@@ -414,7 +416,7 @@
 source_bus_irq = irq,
 dest_io_apic_id = io_apic.id,
 dest_io_apic_intin = 0)
-self.intel_mp_table.add_entry(assign_8259_to_apic)
+base_entries.append(assign_8259_to_apic)
 assign_to_apic = X86IntelMPIOIntAssignment(
 interrupt_type = 'INT',
 polarity = 'ConformPolarity',
@@ -423,11 +425,13 @@
 source_bus_irq = irq,
 dest_io_apic_id = io_apic.id,
 dest_io_apic_intin = apicPin)
-self.intel_mp_table.add_entry(assign_to_apic)
+base_entries.append(assign_to_apic)
 assignISAInt(0, 2)
 assignISAInt(1, 1)
 for i in range(3, 15):
 assignISAInt(i, i)
+self.intel_mp_table.base_entries = base_entries
+self.intel_mp_table.ext_entries = ext_entries
 
 def setWorkCountOptions(system, options):
 if options.work_item_id != None:
@@ -456,17 +460,15 @@
 # just to avoid corner cases.
 assert(self.physmem.range.second.getValue() >= 0x20)
 
-# Mark the first megabyte of memory as reserved
-self.e820_table.entries.append(X86E820Entry(
-addr = 0,
-size = '1MB',
-range_type = 2))
-
-# Mark the rest as available
-self.e820_table.entries.append(X86E820Entry(
-addr = 0x10,
+self.e820_table.entries = \
+   [
+# Mark the first megabyte of memory as reserved
+X86E820Entry(addr = 0, size = '1MB', range_type = 2),
+# Mark the rest as ava

[gem5-dev] changeset in m5: config: tweak ruby configs to clean up hierarchy

[Steve Reinhardt]

changeset 19949c6de823 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=19949c6de823
description:
config: tweak ruby configs to clean up hierarchy

Re-enabling implicit parenting (see previous patch) causes current
Ruby config scripts to create some strange hierarchies and generate
several warnings.  This patch makes three general changes to address
these issues.

1. The order of object creation in the ruby config files makes the L1
   caches children of the sequencer rather than the controller; these
   config ciles are rewritten to assign the L1 caches to the
   controller first.

2. The assignment of the sequencer list to system.ruby.cpu_ruby_ports
   causes the sequencers to be children of system.ruby, generating
   warnings because they are already parented to their respective
   controllers.  Changing this attribute to _cpu_ruby_ports fixes this
   because the leading underscore means this is now treated as a plain
   Python attribute rather than a child assignment. As a result, the
   configuration hierarchy changes such that, e.g.,
   system.ruby.cpu_ruby_ports0 becomes system.l1_cntrl0.sequencer.

3. In the topology classes, the routers become children of some random
   internal link node rather than direct children of the topology.
   The topology classes are rewritten to assign the routers to the
   topology object first.

diffstat:

 configs/example/ruby_direct_test.py   |   4 +-
 configs/example/ruby_fs.py|   8 +++---
 configs/example/ruby_mem_test.py  |   6 ++--
 configs/example/ruby_network_test.py  |   2 +-
 configs/example/ruby_random_test.py   |   4 +-
 configs/example/se.py |   6 ++--
 configs/ruby/MESI_CMP_directory.py|  15 ++-
 configs/ruby/MI_example.py|  11 ---
 configs/ruby/MOESI_CMP_directory.py   |  15 ++-
 configs/ruby/MOESI_CMP_token.py   |  21 ---
 configs/ruby/MOESI_hammer.py  |  19 +++--
 configs/ruby/Network_test.py  |  11 ---
 configs/ruby/Ruby.py  |   2 +-
 src/mem/ruby/network/topologies/Crossbar.py   |  17 ++--
 src/mem/ruby/network/topologies/Mesh.py   |  25 +++---
 src/mem/ruby/network/topologies/MeshDirCorners.py |  30 --
 src/mem/ruby/network/topologies/Pt2Pt.py  |  16 ++-
 src/mem/ruby/network/topologies/Torus.py  |  24 ++---
 tests/configs/memtest-ruby.py |   4 +-
 tests/configs/rubytest-ruby.py|   4 +-
 tests/configs/simple-timing-mp-ruby.py|   6 ++--
 tests/configs/simple-timing-ruby.py   |   6 ++--
 22 files changed, 138 insertions(+), 118 deletions(-)

diffs (truncated from 676 to 300 lines):

diff -r 9f34cf472451 -r 19949c6de823 configs/example/ruby_direct_test.py
--- a/configs/example/ruby_direct_test.py   Mon May 23 14:29:08 2011 -0700
+++ b/configs/example/ruby_direct_test.py   Mon May 23 14:29:23 2011 -0700
@@ -99,9 +99,9 @@
 
 system.ruby = Ruby.create_system(options, system)
 
-assert(options.num_cpus == len(system.ruby.cpu_ruby_ports))
+assert(options.num_cpus == len(system.ruby._cpu_ruby_ports))
 
-for ruby_port in system.ruby.cpu_ruby_ports:
+for ruby_port in system.ruby._cpu_ruby_ports:
 #
 # Tie the ruby tester ports to the ruby cpu ports
 #
diff -r 9f34cf472451 -r 19949c6de823 configs/example/ruby_fs.py
--- a/configs/example/ruby_fs.pyMon May 23 14:29:08 2011 -0700
+++ b/configs/example/ruby_fs.pyMon May 23 14:29:23 2011 -0700
@@ -128,11 +128,11 @@
 #
 # Tie the cpu ports to the correct ruby system ports
 #
-cpu.icache_port = system.ruby.cpu_ruby_ports[i].port
-cpu.dcache_port = system.ruby.cpu_ruby_ports[i].port
+cpu.icache_port = system.ruby._cpu_ruby_ports[i].port
+cpu.dcache_port = system.ruby._cpu_ruby_ports[i].port
 if buildEnv['TARGET_ISA'] == "x86":
-cpu.itb.walker.port = system.ruby.cpu_ruby_ports[i].port
-cpu.dtb.walker.port = system.ruby.cpu_ruby_ports[i].port
+cpu.itb.walker.port = system.ruby._cpu_ruby_ports[i].port
+cpu.dtb.walker.port = system.ruby._cpu_ruby_ports[i].port
 cpu.interrupts.pio = system.piobus.port
 cpu.interrupts.int_port = system.piobus.port
 
diff -r 9f34cf472451 -r 19949c6de823 configs/example/ruby_mem_test.py
--- a/configs/example/ruby_mem_test.py  Mon May 23 14:29:08 2011 -0700
+++ b/configs/example/ruby_mem_test.py  Mon May 23 14:29:23 2011 -0700
@@ -126,20 +126,20 @@
 #
 system.ruby.randomization = True
  
-assert(len(cpus) == len(system.ruby.cpu_ruby_ports))
+assert(len(cpus) == len(syst

[gem5-dev] changeset in m5: config: reinstate implicit parenting on paramet...

[Steve Reinhardt]

changeset 9f34cf472451 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=9f34cf472451
description:
config: reinstate implicit parenting on parameter assignment
Last summer's big rewrite of the initialization code (in
particular cset 6efc3672733b) got rid of the implicit parenting
that used to occur when an unparented SimObject was assigned as
a parameter value to another SimObject.  The idea was that the
new adoptOrphanParams() step would catch these anyway so it was
unnecessary.

Unfortunately it turns out that adoptOrphanParams() has some
inherent instability in that the parent that does the adoption
depends on the config tree traversal order.  Even making this
order deterministic (e.g., by traversing children in
alphabetical order) can introduce unwanted and unexpected
hierarchy changes between similar configs (e.g., when adding a
switch_cpu in place of a cpu), causing problems when trying to
restore checkpoints across similar configs.  The hierarchy
created by implicit parenting is more stable and more
controllable, so this patch turns that behavior back on.

This patch also cleans up some long-standing holes regarding
parenting of SimObjects that are created in class definitions
(either in the body of the class, or as default parameters).

To avoid breaking some existing config files, this necessitated
changing the error on reparenting children to a warning.  This
change fixes another bug where attempting to print the prior
error message would fail on reparenting SimObjectVectors
because they lack a _parent attribute.  Some further issues
with SimObjectVectors were cleaned up by getting rid of the
get_parent() call (which could cause errors with some
SimObjectVectors where there was no single parent to return)
with has_parent() (since all the uses of get_parent() were just
boolean tests anyway).

Finally, since the adoptOrphanParam() step turned out to be so
problematic, we now issue a warning when it actually has to do
an adoption.  Future cleanup of config files will get rid of
current warnings.

diffstat:

 src/python/m5/SimObject.py |  57 ++---
 src/python/m5/params.py|   8 +
 2 files changed, 40 insertions(+), 25 deletions(-)

diffs (145 lines):

diff -r c03e683e83fe -r 9f34cf472451 src/python/m5/SimObject.py
--- a/src/python/m5/SimObject.pyMon May 23 14:27:20 2011 -0700
+++ b/src/python/m5/SimObject.pyMon May 23 14:29:08 2011 -0700
@@ -278,12 +278,26 @@
 def _set_param(cls, name, value, param):
 assert(param.name == name)
 try:
-cls._values[name] = param.convert(value)
+value = param.convert(value)
 except Exception, e:
 msg = "%s\nError setting param %s.%s to %s\n" % \
   (e, cls.__name__, name, value)
 e.args = (msg, )
 raise
+cls._values[name] = value
+# if param value is a SimObject, make it a child too, so that
+# it gets cloned properly when the class is instantiated
+if isSimObjectOrVector(value) and not value.has_parent():
+cls._add_cls_child(name, value)
+
+def _add_cls_child(cls, name, child):
+# It's a little funky to have a class as a parent, but these
+# objects should never be instantiated (only cloned, which
+# clears the parent pointer), and this makes it clear that the
+# object is not an orphan and can provide better error
+# messages.
+child.set_parent(cls, name)
+cls._children[name] = child
 
 def _new_port(cls, name, port):
 # each port should be uniquely assigned to one variable
@@ -334,7 +348,7 @@
 
 if isSimObjectOrSequence(value):
 # If RHS is a SimObject, it's an implicit child assignment.
-cls._children[attr] = coerceSimObjectOrVector(value)
+cls._add_cls_child(attr, coerceSimObjectOrVector(value))
 return
 
 # no valid assignment... raise exception
@@ -508,6 +522,14 @@
 self._ccParams = None
 self._instantiated = False # really "cloned"
 
+# Clone children specified at class level.  No need for a
+# multidict here since we will be cloning everything.
+# Do children before parameter values so that children that
+# are also param values get cloned properly.
+self._children = {}
+for key,val in ancestor._children.iteritems():
+self.add_child(key, val(_memo=memo_dict))
+
 # Inherit parameter values from class using multidict so
 # individual value settings can be overridden but we still
 # inherit late changes to non-overridden class values.
@@ -518

[gem5-dev] changeset in m5: util/regress: make default action a more thorou...

[Steve Reinhardt]

changeset 6a49ac49fd67 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=6a49ac49fd67
description:
util/regress: make default action a more thorough regression

Changed the --variants option to --test-variants and added a new
--compile-variants option for variants that are only compiled
(not tested).  The former still defaults to 'opt' and the latter
defaults to 'debug,fast'.

Also changed the behavior when no tests are specified from just
compiling to running the 'quick' tests.

As a result, a plain 'util/regress' invocation will now compile
(but not test) the debug and fast builds, and compile and run the
quick regressions on the opt build.  This should be the default
set of tests that are run before committing.  Since the nightly
regressions use this same script, this will also be the new
nightly regression behavior.

Test-only regressions can still be done by setting --compile=''.
Compile-only regressions can be done by setting --test=''.

diffstat:

 util/regress |  78 ---
 1 files changed, 48 insertions(+), 30 deletions(-)

diffs (126 lines):

diff -r 3f37cc5d25bc -r 6a49ac49fd67 util/regress
--- a/util/regress  Mon May 23 14:36:22 2011 -0400
+++ b/util/regress  Mon May 23 14:27:20 2011 -0700
@@ -37,10 +37,9 @@
 
 optparser = optparse.OptionParser()
 add_option = optparser.add_option
-add_option('-v', '--verbose', dest='verbose', action='store_true',
-   default=False,
+add_option('-v', '--verbose', action='store_true', default=False,
help='echo commands before executing')
-add_option('--builds', dest='builds',
+add_option('--builds',
default='ALPHA_SE,ALPHA_SE_MOESI_hammer,' \
'ALPHA_SE_MESI_CMP_directory,'  \
'ALPHA_SE_MOESI_CMP_directory,' \
@@ -52,15 +51,21 @@
'X86_SE,X86_FS,' \
'ARM_SE,ARM_FS',
help="comma-separated build targets to test (default: '%default')")
-add_option('--variants', dest='variants', default='opt',
-   help="comma-separated build variants to test (default: '%default')")
-add_option('--scons-opts', dest='scons_opts', default='', metavar='OPTS',
+add_option('--test-variants', default='opt',
+   help="comma-separated build variants to test (default: '%default')"\
+   ", set to '' for none")
+add_option('--compile-variants', default='debug,fast',
+   help="comma-separated build variants to compile only (not test) " \
+   "(default: '%default'), set to '' for none", metavar='VARIANTS')
+add_option('--scons-opts', default='', metavar='OPTS',
help='scons options')
-add_option('-j', '--jobs', type='int', default=1,
-   help='number of parallel jobs to use')
+add_option('-j', '--jobs', type='int', default=1, metavar='N',
+   help='number of parallel jobs to use (0 to use all cores)')
 add_option('-k', '--keep-going', action='store_true',
help='keep going after errors')
-add_option('-D', '--build-dir', default='',
+add_option('--update-ref', action='store_true',
+   help='update reference outputs')
+add_option('-D', '--build-dir', default='', metavar='DIR',
help='build directory location')
 add_option('-n', "--no-exec", default=False, action='store_true',
help="don't actually invoke scons, just echo SCons command line")
@@ -68,9 +73,17 @@
 (options, tests) = optparser.parse_args()
 
 
+# split a comma-separated list, but return an empty list if given the
+# empty string
+def split_if_nonempty(s):
+if not s:
+return []
+return s.split(',')
+
 # split list options on ',' to get Python lists
-builds = options.builds.split(',')
-variants = options.variants.split(',')
+builds = split_if_nonempty(options.builds)
+test_variants = split_if_nonempty(options.test_variants)
+compile_variants = split_if_nonempty(options.compile_variants)
 
 options.build_dir = os.path.join(options.build_dir, 'build')
 
@@ -91,30 +104,33 @@
 print >>sys.stderr, "When attemping to execute: %s" % cmd
 sys.exit(1)
 
-# Quote string s so it can be passed as a shell arg
-def shellquote(s):
-if ' ' in s:
-s = "'%s'" % s
-return s
+targets = []
 
+# start with compile-only targets, if any
+if compile_variants:
+targets += ['%s/%s/m5.%s' % (options.build_dir, build, variant)
+for variant in compile_variants
+for build in builds]
+
+# By default run the 'quick' tests
 if not tests:
-print "No tests specified, just building binaries."
-targets = ['%s/%s/m5.%s' % (options.build_dir, build, variant)
-   for build in builds
-   for variant in variants]
-elif 'all' in tests:
-targets = ['%s/%s/tests/%s' % (options.build_dir, build, variant)
-   for build in builds
-   for variant in variant

[gem5-dev] changeset in m5: sim: add some DPRINTFs for debugging unserializ...

[Steve Reinhardt]

changeset c03e683e83fe in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=c03e683e83fe
description:
sim: add some DPRINTFs for debugging unserialization
Also got rid of unused C++ unserializeAll() method
(this is now handled in Python)

diffstat:

 src/sim/sim_object.cc |  27 ++-
 src/sim/sim_object.hh |   1 -
 2 files changed, 6 insertions(+), 22 deletions(-)

diffs (63 lines):

diff -r 6a49ac49fd67 -r c03e683e83fe src/sim/sim_object.cc
--- a/src/sim/sim_object.cc Mon May 23 14:27:20 2011 -0700
+++ b/src/sim/sim_object.cc Mon May 23 14:27:20 2011 -0700
@@ -38,7 +38,7 @@
 #include "base/misc.hh"
 #include "base/trace.hh"
 #include "base/types.hh"
-#include "debug/Config.hh"
+#include "debug/Checkpoint.hh"
 #include "sim/sim_object.hh"
 #include "sim/stats.hh"
 
@@ -78,8 +78,12 @@
 void
 SimObject::loadState(Checkpoint *cp)
 {
-if (cp->sectionExists(name()))
+if (cp->sectionExists(name())) {
+DPRINTF(Checkpoint, "unserializing\n");
 unserialize(cp, name());
+} else {
+DPRINTF(Checkpoint, "no checkpoint section found\n");
+}
 }
 
 void
@@ -126,25 +130,6 @@
}
 }
 
-void
-SimObject::unserializeAll(Checkpoint *cp)
-{
-SimObjectList::reverse_iterator ri = simObjectList.rbegin();
-SimObjectList::reverse_iterator rend = simObjectList.rend();
-
-for (; ri != rend; ++ri) {
-SimObject *obj = *ri;
-DPRINTFR(Config, "Unserializing '%s'\n",
- obj->name());
-if(cp->sectionExists(obj->name()))
-obj->unserialize(cp, obj->name());
-else
-warn("Not unserializing '%s': no section found in checkpoint.\n",
- obj->name());
-   }
-}
-
-
 
 #ifdef DEBUG
 //
diff -r 6a49ac49fd67 -r c03e683e83fe src/sim/sim_object.hh
--- a/src/sim/sim_object.hh Mon May 23 14:27:20 2011 -0700
+++ b/src/sim/sim_object.hh Mon May 23 14:27:20 2011 -0700
@@ -138,7 +138,6 @@
 
 // static: call nameOut() & serialize() on all SimObjects
 static void serializeAll(std::ostream &);
-static void unserializeAll(Checkpoint *cp);
 
 // Methods to drain objects in order to take checkpoints
 // Or switch from timing -> atomic memory model
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Review Request: syscall emul: fix Power Linux mmap constant, plus other cleanup

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/710/
---

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

syscall emul: fix Power Linux mmap constant, plus other cleanup

We were getting a spurious warning in the regressions that turned
out to be due to having the wrong value for TGT_MAP_ANONYMOUS for
Power Linux, but in the process of tracking it down I ended up
doing some cleanup of the mmap handling in general.


Diffs
-

  src/arch/power/linux/linux.hh 3f37cc5d25bc 
  src/sim/process.cc 3f37cc5d25bc 
  src/sim/syscall_emul.hh 3f37cc5d25bc 

Diff: http://reviews.m5sim.org/r/710/diff


Testing
---


Thanks,

Steve

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Auto-generated error/warning URLs

[Steve Reinhardt]

You know the "For more  information see:
http://www.m5sim.org/warn/3a2134f6"; URLs?  How many of these are
actually used?  They seem mostly like noise to me.  If we're not using
them, can we get rid of them?  Seems like it's just as intuitive to
search for the warning/error string on the wiki (or in the mailing
list).

Thoughts?

Steve
___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Review Request: config: tweak ruby configs to clean up hierarchy

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/709/
---

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

config: tweak ruby configs to clean up hierarchy

Re-enabling implicit parenting (see previous patch) causes current
Ruby config scripts to create some strange hierarchies and generate
several warnings.  This patch makes three general changes to address
these issues.

1. The order of object creation in the ruby config files makes the L1
   caches children of the sequencer rather than the controller; these
   config ciles are rewritten to assign the L1 caches to the
   controller first.

2. The assignment of the sequencer list to system.ruby.cpu_ruby_ports
   causes the sequencers to be children of system.ruby, generating
   warnings because they are already parented to their respective
   controllers.  Changing this attribute to _cpu_ruby_ports fixes this
   because the leading underscore means this is now treated as a plain
   Python attribute rather than a child assignment. As a result, the
   configuration hierarchy changes such that, e.g.,
   system.ruby.cpu_ruby_ports0 becomes system.l1_cntrl0.sequencer.

3. In the topology classes, the routers become children of some random
   internal link node rather than direct children of the topology.
   The topology classes are rewritten to assign the routers to the
   topology object first.

[Note: the "previous patch" referred to above is http://reviews.m5sim.org/r/608 
,
which will be the previous patch when I commit these.]


Diffs
-

  configs/example/ruby_direct_test.py 7f106d0bd638 
  configs/example/ruby_fs.py 7f106d0bd638 
  configs/example/ruby_mem_test.py 7f106d0bd638 
  configs/example/ruby_network_test.py 7f106d0bd638 
  configs/example/ruby_random_test.py 7f106d0bd638 
  configs/example/se.py 7f106d0bd638 
  configs/ruby/MESI_CMP_directory.py 7f106d0bd638 
  configs/ruby/MI_example.py 7f106d0bd638 
  configs/ruby/MOESI_CMP_directory.py 7f106d0bd638 
  configs/ruby/MOESI_CMP_token.py 7f106d0bd638 
  configs/ruby/MOESI_hammer.py 7f106d0bd638 
  configs/ruby/Ruby.py 7f106d0bd638 
  src/mem/ruby/network/topologies/Crossbar.py 7f106d0bd638 
  src/mem/ruby/network/topologies/Mesh.py 7f106d0bd638 
  src/mem/ruby/network/topologies/MeshDirCorners.py 7f106d0bd638 
  src/mem/ruby/network/topologies/Pt2Pt.py 7f106d0bd638 
  src/mem/ruby/network/topologies/Torus.py 7f106d0bd638 
  tests/configs/memtest-ruby.py 7f106d0bd638 
  tests/configs/rubytest-ruby.py 7f106d0bd638 
  tests/configs/simple-timing-mp-ruby.py 7f106d0bd638 
  tests/configs/simple-timing-ruby.py 7f106d0bd638 

Diff: http://reviews.m5sim.org/r/709/diff


Testing
---


Thanks,

Steve

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: util/regress: make default action a more thorough regression

[Steve Reinhardt]

> On 2011-04-21 13:37:08, Nathan Binkert wrote:
> > util/regress, line 76
> > <http://reviews.m5sim.org/r/649/diff/1/?file=11693#file11693line76>
> >
> > I have nicer code for this sort of thing.  You can see it in 
> > src/python/m5/options.py, examples of usage in main.py
> > 
> > Look for action='append' (and notice the necessity of split)
> 
> Steve Reinhardt wrote:
> So I played around with this, and the first problem is that your current 
> code can't handle default values.  I fixed that in options.py, but it still 
> doesn't print the defaults nicely, e.g., instead of:
> 
>   --test-variants=TEST_VARIANTS
> comma-separated build variants to test (default:
> 'opt')
>   --compile-variants=VARIANTS
> comma-separated build variants to compile only 
> (not
> test) (default: 'debug,fast')
> 
> we get:
> 
> --test-variants=TEST_VARIANTS
> comma-separated build variants to test (default:
> '['opt']')
> --compile-variants=VARIANTS
> comma-separated build variants to compile only 
> (not
> test) (default: '['debug', 'fast']')
> 
> Not tragic, but I like the former better, so I may keep the original 
> solution.
>

On further consideration, 'append' is not even what I want, if the user gives a 
list I want it to replace (not append to) any existing list.


- Steve


-------
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/649/#review1140
---


On 2011-04-20 22:42:16, Steve Reinhardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/649/
> ---
> 
> (Updated 2011-04-20 22:42:16)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> util/regress: make default action a more thorough regression
> 
> Changed the --variants option to --test-variants and added a new
> --compile-variants option for variants that are only compiled
> (not tested).  The former still defaults to 'opt' and the latter
> defaults to 'debug,fast'.
> 
> Also changed the behavior when no tests are specified from just
> compiling to running the 'quick' tests.
> 
> As a result, a plain 'util/regress' invocation will now compile
> (but not test) the debug and fast builds, and compile and run the
> quick regressions on the opt build.  This should be the default
> set of tests that are run before committing.  Since the nightly
> regressions use this same script, this will also be the new
> nightly regression behavior.
> 
> Test-only regressions can still be done by setting --compile=''.
> Compile-only regressions can be done by setting --test=''.
> 
> 
> Diffs
> -
> 
>   util/regress a9d06c894afe 
> 
> Diff: http://reviews.m5sim.org/r/649/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request: util/regress: make default action a more thorough regression

[Steve Reinhardt]

> On 2011-04-21 13:37:08, Nathan Binkert wrote:
> > util/regress, line 76
> > <http://reviews.m5sim.org/r/649/diff/1/?file=11693#file11693line76>
> >
> > I have nicer code for this sort of thing.  You can see it in 
> > src/python/m5/options.py, examples of usage in main.py
> > 
> > Look for action='append' (and notice the necessity of split)

So I played around with this, and the first problem is that your current code 
can't handle default values.  I fixed that in options.py, but it still doesn't 
print the defaults nicely, e.g., instead of:

  --test-variants=TEST_VARIANTS
comma-separated build variants to test (default:
'opt')
  --compile-variants=VARIANTS
comma-separated build variants to compile only (not
test) (default: 'debug,fast')

we get:

--test-variants=TEST_VARIANTS
comma-separated build variants to test (default:
'['opt']')
--compile-variants=VARIANTS
comma-separated build variants to compile only (not
test) (default: '['debug', 'fast']')

Not tragic, but I like the former better, so I may keep the original solution.


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/649/#review1140
---


On 2011-04-20 22:42:16, Steve Reinhardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/649/
> -------
> 
> (Updated 2011-04-20 22:42:16)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> util/regress: make default action a more thorough regression
> 
> Changed the --variants option to --test-variants and added a new
> --compile-variants option for variants that are only compiled
> (not tested).  The former still defaults to 'opt' and the latter
> defaults to 'debug,fast'.
> 
> Also changed the behavior when no tests are specified from just
> compiling to running the 'quick' tests.
> 
> As a result, a plain 'util/regress' invocation will now compile
> (but not test) the debug and fast builds, and compile and run the
> quick regressions on the opt build.  This should be the default
> set of tests that are run before committing.  Since the nightly
> regressions use this same script, this will also be the new
> nightly regression behavior.
> 
> Test-only regressions can still be done by setting --compile=''.
> Compile-only regressions can be done by setting --test=''.
> 
> 
> Diffs
> -
> 
>   util/regress a9d06c894afe 
> 
> Diff: http://reviews.m5sim.org/r/649/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve
> 
>

___
gem5-dev mailing list
gem5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [m5-dev] Review Request: O3: Removed unnecessary unserialize instruction flags.

[Steve Reinhardt]

> On 2011-05-13 14:38:59, Nathan Binkert wrote:
> > Really?  Are you sure that really works?  Those registers are not renamed 
> > and things could go wrong.

FPCR should be fine, as the manual requires you to surround it with EXCB (which 
is still serializing) if you want precise information.

rduniq/wruniq I'm less sure about... seems like it should be sufficient to only 
serialize wruniq though, since the only time you'll have problems with rduniq 
is if it gets reordered wrt a wruniq.


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/694/#review1233
---


On 2011-05-13 14:34:53, Yasuko Watanabe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/694/
> ---
> 
> (Updated 2011-05-13 14:34:53)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> O3: Removed unnecessary unserialize instruction flags.
> 
> 
> Diffs
> -
> 
>   src/arch/alpha/isa/decoder.isa 54a65799e4c1 
> 
> Diff: http://reviews.m5sim.org/r/694/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yasuko
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] src/dest detection in the ISA descriptions

[Steve Reinhardt]

On Wed, May 4, 2011 at 2:25 PM, Gabe Black  wrote:

> Did that make sense?


I see how that could work... I think I was more puzzled by how you would
figure out that

for (int i = 0; i < 7; i++)
Dest.bytes[i] = Source1.bytes[i] + Source2.bytes[i];

overwrote all of Dest, but

for (int i = 0; i < 4; i++)
Dest.bytes[i] = Source1.bytes[i] + Source2.bytes[i];

wouldn't... but looking back I see now that you'd expect to need manual
annotations in at least one of those cases.


> Do you think you'll be able to review those patches
> soonish?
>

I'll try... thanks for the reminder, that definitely increases the
probability :-).

Steve
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] changeset in m5: debug: fix help output

[Steve Reinhardt]

So --trace-start, --trace-ignore, and --trace-file didn't get renamed?

On Wed, May 4, 2011 at 4:21 PM, nathan binkert  wrote:

> I'm not sure.  Something must have happened when I rebased my patch
> queue.  I believe all that needs to happen is delete the 4 lines
> relating to --trace-help and --trace-flags.  Can someone delete them
> for me and make sure that M5 still runs correctly?  I'm out of town
> and I'm really not going to be able to do anything for a couple of
> days.
>
>  Nate
>
> > Note that running "m5 --help" still has the old names:
> >
> > Trace Options
> > -
> > --trace-helpPrint help on trace flags
> > --trace-flags=FLAG[,FLAG]
> >Sets the flags for tracing (-FLAG disables a flag)
> > --trace-start=TIME  Start tracing at TIME (must be in ticks)
> > --trace-file=FILE   Sets the output file for tracing [Default: cout]
> > --trace-ignore=EXPR Ignore EXPR sim objects
> >
> >
> > On Wed, May 4, 2011 at 7:07 AM, Nathan Binkert  wrote:
> >
> >> changeset 5a9a639ce16f in /z/repo/m5
> >> details: http://repo.m5sim.org/m5?cmd=changeset;node=5a9a639ce16f
> >> description:
> >>debug: fix help output
> >>
> >> diffstat:
> >>
> >>  src/base/debug.cc  |   4 ++--
> >>  src/base/debug.hh  |  16 
> >>  src/python/m5/debug.py |  28 
> >>  3 files changed, 30 insertions(+), 18 deletions(-)
> >>
> >> diffs (115 lines):
> >>
> >> diff -r 3f49ed206f46 -r 5a9a639ce16f src/base/debug.cc
> >> --- a/src/base/debug.cc Mon May 02 12:40:32 2011 -0700
> >> +++ b/src/base/debug.cc Wed May 04 10:08:08 2011 -0400
> >> @@ -101,14 +101,14 @@
> >>  CompoundFlag::enable()
> >>  {
> >> SimpleFlag::enable();
> >> -for_each(flags.begin(), flags.end(), mem_fun(&Flag::enable));
> >> +for_each(_kids.begin(), _kids.end(), mem_fun(&Flag::enable));
> >>  }
> >>
> >>  void
> >>  CompoundFlag::disable()
> >>  {
> >> SimpleFlag::disable();
> >> -for_each(flags.begin(), flags.end(), mem_fun(&Flag::disable));
> >> +for_each(_kids.begin(), _kids.end(), mem_fun(&Flag::disable));
> >>  }
> >>
> >>  struct AllFlags : public Flag
> >> diff -r 3f49ed206f46 -r 5a9a639ce16f src/base/debug.hh
> >> --- a/src/base/debug.hh Mon May 02 12:40:32 2011 -0700
> >> +++ b/src/base/debug.hh Wed May 04 10:08:08 2011 -0400
> >> @@ -44,6 +44,7 @@
> >>   protected:
> >> const char *_name;
> >> const char *_desc;
> >> +std::vector _kids;
> >>
> >>   public:
> >> Flag(const char *name, const char *desc);
> >> @@ -51,6 +52,7 @@
> >>
> >> std::string name() const { return _name; }
> >> std::string desc() const { return _desc; }
> >> +std::vector kids() { return _kids; }
> >>
> >> virtual void enable() = 0;
> >> virtual void disable() = 0;
> >> @@ -77,7 +79,12 @@
> >>  class CompoundFlag : public SimpleFlag
> >>  {
> >>   protected:
> >> -std::vector flags;
> >> +void
> >> +addFlag(Flag &f)
> >> +{
> >> +if (&f != NULL)
> >> +_kids.push_back(&f);
> >> +}
> >>
> >>   public:
> >> CompoundFlag(const char *name, const char *desc,
> >> @@ -99,13 +106,6 @@
> >> addFlag(f15); addFlag(f16); addFlag(f17); addFlag(f18);
> >> addFlag(f19);
> >> }
> >>
> >> -void
> >> -addFlag(Flag &f)
> >> -{
> >> -if (&f != NULL)
> >> -flags.push_back(&f);
> >> -}
> >> -
> >> void enable();
> >> void disable();
> >>  };
> >> diff -r 3f49ed206f46 -r 5a9a639ce16f src/python/m5/debug.py
> >> --- a/src/python/m5/debug.pyMon May 02 12:40:32 2011 -0700
> >> +++ b/src/python/m5/debug.pyWed May 04 10:08:08 2011 -0400
> >> @@ -26,24 +26,36 @@
> >>  #
> >>  # Authors: Nathan Binkert
> >>
> >> +from UserDict import DictMixin
> >> +
> >>  import internal
> >>
> >> +from internal.debug import SimpleFlag, CompoundFlag
> >>  from internal.debug import schedBreakCycle, setRemoteGDBPort
> >> +from m5.util import printList
> >>
> >>  def help():
> >> print "Base Flags:"
> >> -for flag in flags.basic:
> >> -print "%s: %s" % (flag, flags.descriptions[flag])
> >> +for name in sorted(flags):
> >> +if name == 'All':
> >> +continue
> >> +flag = flags[name]
> >> +children = [c for c in flag.kids() ]
> >> +if not children:
> >> +print "%s: %s" % (name, flag.desc())
> >> print
> >> print "Compound Flags:"
> >> -for flag in flags.compound:
> >> -if flag == 'All':
> >> +for name in sorted(flags):
> >> +if name == 'All':
> >> continue
> >> -print "%s: %s" % (flag, flags.descriptions[flag])
> >> -util.printList(flags.compoundMap[flag], indent=8)
> >> -print
> >> +flag = flags[name]
> >> +children = [c for c in flag.kids() ]
> >> +if children:
> >> +print "%s: %s" % (name, flag.desc())
> >> +printList([ c.name() for c in children ], inden

Re: [m5-dev] changeset in m5: debug: fix help output

[Steve Reinhardt]

Note that running "m5 --help" still has the old names:

Trace Options
-
--trace-helpPrint help on trace flags
--trace-flags=FLAG[,FLAG]
Sets the flags for tracing (-FLAG disables a flag)
--trace-start=TIME  Start tracing at TIME (must be in ticks)
--trace-file=FILE   Sets the output file for tracing [Default: cout]
--trace-ignore=EXPR Ignore EXPR sim objects


On Wed, May 4, 2011 at 7:07 AM, Nathan Binkert  wrote:

> changeset 5a9a639ce16f in /z/repo/m5
> details: http://repo.m5sim.org/m5?cmd=changeset;node=5a9a639ce16f
> description:
>debug: fix help output
>
> diffstat:
>
>  src/base/debug.cc  |   4 ++--
>  src/base/debug.hh  |  16 
>  src/python/m5/debug.py |  28 
>  3 files changed, 30 insertions(+), 18 deletions(-)
>
> diffs (115 lines):
>
> diff -r 3f49ed206f46 -r 5a9a639ce16f src/base/debug.cc
> --- a/src/base/debug.cc Mon May 02 12:40:32 2011 -0700
> +++ b/src/base/debug.cc Wed May 04 10:08:08 2011 -0400
> @@ -101,14 +101,14 @@
>  CompoundFlag::enable()
>  {
> SimpleFlag::enable();
> -for_each(flags.begin(), flags.end(), mem_fun(&Flag::enable));
> +for_each(_kids.begin(), _kids.end(), mem_fun(&Flag::enable));
>  }
>
>  void
>  CompoundFlag::disable()
>  {
> SimpleFlag::disable();
> -for_each(flags.begin(), flags.end(), mem_fun(&Flag::disable));
> +for_each(_kids.begin(), _kids.end(), mem_fun(&Flag::disable));
>  }
>
>  struct AllFlags : public Flag
> diff -r 3f49ed206f46 -r 5a9a639ce16f src/base/debug.hh
> --- a/src/base/debug.hh Mon May 02 12:40:32 2011 -0700
> +++ b/src/base/debug.hh Wed May 04 10:08:08 2011 -0400
> @@ -44,6 +44,7 @@
>   protected:
> const char *_name;
> const char *_desc;
> +std::vector _kids;
>
>   public:
> Flag(const char *name, const char *desc);
> @@ -51,6 +52,7 @@
>
> std::string name() const { return _name; }
> std::string desc() const { return _desc; }
> +std::vector kids() { return _kids; }
>
> virtual void enable() = 0;
> virtual void disable() = 0;
> @@ -77,7 +79,12 @@
>  class CompoundFlag : public SimpleFlag
>  {
>   protected:
> -std::vector flags;
> +void
> +addFlag(Flag &f)
> +{
> +if (&f != NULL)
> +_kids.push_back(&f);
> +}
>
>   public:
> CompoundFlag(const char *name, const char *desc,
> @@ -99,13 +106,6 @@
> addFlag(f15); addFlag(f16); addFlag(f17); addFlag(f18);
> addFlag(f19);
> }
>
> -void
> -addFlag(Flag &f)
> -{
> -if (&f != NULL)
> -flags.push_back(&f);
> -}
> -
> void enable();
> void disable();
>  };
> diff -r 3f49ed206f46 -r 5a9a639ce16f src/python/m5/debug.py
> --- a/src/python/m5/debug.pyMon May 02 12:40:32 2011 -0700
> +++ b/src/python/m5/debug.pyWed May 04 10:08:08 2011 -0400
> @@ -26,24 +26,36 @@
>  #
>  # Authors: Nathan Binkert
>
> +from UserDict import DictMixin
> +
>  import internal
>
> +from internal.debug import SimpleFlag, CompoundFlag
>  from internal.debug import schedBreakCycle, setRemoteGDBPort
> +from m5.util import printList
>
>  def help():
> print "Base Flags:"
> -for flag in flags.basic:
> -print "%s: %s" % (flag, flags.descriptions[flag])
> +for name in sorted(flags):
> +if name == 'All':
> +continue
> +flag = flags[name]
> +children = [c for c in flag.kids() ]
> +if not children:
> +print "%s: %s" % (name, flag.desc())
> print
> print "Compound Flags:"
> -for flag in flags.compound:
> -if flag == 'All':
> +for name in sorted(flags):
> +if name == 'All':
> continue
> -print "%s: %s" % (flag, flags.descriptions[flag])
> -util.printList(flags.compoundMap[flag], indent=8)
> -print
> +flag = flags[name]
> +children = [c for c in flag.kids() ]
> +if children:
> +print "%s: %s" % (name, flag.desc())
> +printList([ c.name() for c in children ], indent=8)
> +print
>
> -class AllFlags(object):
> +class AllFlags(DictMixin):
> def __init__(self):
> self._version = -1
> self._dict = {}
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: SConstruct: automatically update .hg/hgrc with style hooks

[Steve Reinhardt]

> On 2011-05-02 12:51:56, Nathan Binkert wrote:
> > SConstruct, line 247
> > <http://reviews.m5sim.org/r/668/diff/1/?file=12211#file12211line247>
> >
> > Should we prompt the user for permission?  That way the user would at 
> > least know that something happened.
> 
> Steve Reinhardt wrote:
> I doubt most people care.  How many would really say no?  Most people 
> just want things to be automatic.
> 
> If they really do care they can always edit it out later (the comments 
> will let them know where the hooks came from).
>
> 
> Nathan Binkert wrote:
> I agree that nobody would say no, but most people also would never notice 
> that it had happened because the message would just fly by if there was no 
> prompt.  I guess that you're arguing that that would be a good thing :)
> 
> Gabe Black wrote:
> Changing config files behind peoples back is a really bad idea in my 
> opinion. I know I've stopped using entire distros (Suse) because they mucked 
> with config files behind my back and perpetually broke my system. My configs 
> are mine, and the minimal level of respect for that would be if we asked 
> permission before we let ourselves in. I think leaving well enough alone and 
> having the user go in and fix it themselves is actually the best approach.
> 
> Nathan Binkert wrote:
> This isn't a config file in your homedir, it's the config file in the M5 
> repo and it will only be changed if you don't have it set up right, so it's 
> not all that bad.  I have the same leaning that you do, so I think I'd rather 
> see a prompt for the user, but I don't feel that strongly about it.

Yea, I would feel differently about ~/.hgrc, but this is just a local config 
file, and it's one that will need updating each time you clone a new repo.  I 
don't mind making the warning a little more prominent, but since the status quo 
is that we won't even try to compile if the hooks aren't there, it seems 
overkill to me to make too big a deal of it... is someone going to be so 
offended about this that they're going to go try a different simulator?  Plus 
if we could find a way to get these hooks to be part of the m5 repo so that 
they were automatically in place whenever you cloned, we would have done that, 
so I don't see how this is much different.


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/668/#review1173
---


On 2011-05-02 12:34:56, Steve Reinhardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/668/
> ---
> 
> (Updated 2011-05-02 12:34:56)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> SConstruct: automatically update .hg/hgrc with style hooks
> 
> Seems easier than pestering people about it.
> Note also that path is now absolute, so you don't get errors
> when invoking hg from subdirectories.
> 
> 
> Diffs
> -
> 
>   SConstruct 66a3187a6714 
> 
> Diff: http://reviews.m5sim.org/r/668/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: SConstruct: automatically update .hg/hgrc with style hooks

[Steve Reinhardt]

> On 2011-05-02 12:51:56, Nathan Binkert wrote:
> > SConstruct, line 243
> > <http://reviews.m5sim.org/r/668/diff/1/?file=12211#file12211line243>
> >
> > I know you didn't change this, but should we check for both hooks?  (I 
> > added the pre-qrefresh one a while ago)
> 
> Steve Reinhardt wrote:
> Doesn't make me much difference... seems pretty unlikely anyone would 
> have one and not the other though.
> 
> Nathan Binkert wrote:
> Old repos may not have the new hook. (I only found out that we could use 
> pre-qrefresh a few months ago).

The only hitch is that if we just append the hook text when only one is missing 
we'll get duplicates, like this:

[extensions]
style = /home/stever/hg/amd/m5/util/style.py

[hooks]
pretxncommit.style = python:style.check_style

# The following lines were automatically added by m5/SConstruct
# to provide the m5 style-checking hooks
[extensions]
style = /home/stever/hg/amd/m5/util/style.py

[hooks]
pretxncommit.style = python:style.check_style
pre-qrefresh.style = python:style.check_style
# End of SConstruct additions

I don't think it's a problem (my very limited testing hasn't shown anything), 
but it is a little weird.  If you think this is OK, then I'll go ahead and test 
for both.  Otherwise I could test for both, print a warning if they're only 
half there, and only do the auto append if they're both missing.


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/668/#review1173
---


On 2011-05-02 12:34:56, Steve Reinhardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/668/
> -------
> 
> (Updated 2011-05-02 12:34:56)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> SConstruct: automatically update .hg/hgrc with style hooks
> 
> Seems easier than pestering people about it.
> Note also that path is now absolute, so you don't get errors
> when invoking hg from subdirectories.
> 
> 
> Diffs
> -
> 
>   SConstruct 66a3187a6714 
> 
> Diff: http://reviews.m5sim.org/r/668/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: SConstruct: automatically update .hg/hgrc with style hooks

[Steve Reinhardt]

> On 2011-05-02 12:51:56, Nathan Binkert wrote:
> > SConstruct, line 243
> > <http://reviews.m5sim.org/r/668/diff/1/?file=12211#file12211line243>
> >
> > I know you didn't change this, but should we check for both hooks?  (I 
> > added the pre-qrefresh one a while ago)

Doesn't make me much difference... seems pretty unlikely anyone would have one 
and not the other though.


> On 2011-05-02 12:51:56, Nathan Binkert wrote:
> > SConstruct, line 247
> > <http://reviews.m5sim.org/r/668/diff/1/?file=12211#file12211line247>
> >
> > Should we prompt the user for permission?  That way the user would at 
> > least know that something happened.

I doubt most people care.  How many would really say no?  Most people just want 
things to be automatic.

If they really do care they can always edit it out later (the comments will let 
them know where the hooks came from).


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/668/#review1173
-------


On 2011-05-02 12:34:56, Steve Reinhardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/668/
> -------
> 
> (Updated 2011-05-02 12:34:56)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> SConstruct: automatically update .hg/hgrc with style hooks
> 
> Seems easier than pestering people about it.
> Note also that path is now absolute, so you don't get errors
> when invoking hg from subdirectories.
> 
> 
> Diffs
> -
> 
>   SConstruct 66a3187a6714 
> 
> Diff: http://reviews.m5sim.org/r/668/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] changeset in m5: scons: interpret paths relative to launch direc...

[Steve Reinhardt]

changeset 3f49ed206f46 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=3f49ed206f46
description:
scons: interpret paths relative to launch directory

Make sure all command-line targets and EXTRAS directories
are interpreted relative to the launch directory.  This
turns out to be very useful when building code from an
EXTRAS directory using SCons's -C option.

We were trying to do this with targets but it didn't actually
work since we didn't update BUILD_TARGETS (so SCons got
confused internally).  We weren't even trying with EXTRAS.

To simplify the code, the default target is also interpreted
relative to the launch dir even though it was explicitly
handled as relative to the m5 dir before... I doubt anyone
really uses this anyway so it didn't seem worth the complexity.
(Maybe we should get rid of it?)

diffstat:

 SConstruct |  52 +++-
 1 files changed, 19 insertions(+), 33 deletions(-)

diffs (102 lines):

diff -r 06f3a4cbd585 -r 3f49ed206f46 SConstruct
--- a/SConstructMon May 02 12:40:31 2011 -0700
+++ b/SConstructMon May 02 12:40:32 2011 -0700
@@ -264,30 +264,29 @@
 return i
 raise ValueError, "element not found"
 
+# Take a list of paths (or SCons Nodes) and return a list with all
+# paths made absolute and ~-expanded.  Paths will be interpreted
+# relative to the launch directory unless a different root is provided
+def makePathListAbsolute(path_list, root=GetLaunchDir()):
+return [abspath(joinpath(root, expanduser(str(p
+for p in path_list]
+
 # Each target must have 'build' in the interior of the path; the
 # directory below this will determine the build parameters.  For
 # example, for target 'foo/bar/build/ALPHA_SE/arch/alpha/blah.do' we
 # recognize that ALPHA_SE specifies the configuration because it
-# follow 'build' in the bulid path.
+# follow 'build' in the build path.
 
-# Generate absolute paths to targets so we can see where the build dir is
-if COMMAND_LINE_TARGETS:
-# Ask SCons which directory it was invoked from
-launch_dir = GetLaunchDir()
-# Make targets relative to invocation directory
-abs_targets = [ normpath(joinpath(launch_dir, str(x))) for x in \
-COMMAND_LINE_TARGETS]
-else:
-# Default targets are relative to root of tree
-abs_targets = [ normpath(joinpath(main.root.abspath, str(x))) for x in \
-DEFAULT_TARGETS]
-
+# The funky assignment to "[:]" is needed to replace the list contents
+# in place rather than reassign the symbol to a new list, which
+# doesn't work (obviously!).
+BUILD_TARGETS[:] = makePathListAbsolute(BUILD_TARGETS)
 
 # Generate a list of the unique build roots and configs that the
 # collected targets reference.
 variant_paths = []
 build_root = None
-for t in abs_targets:
+for t in BUILD_TARGETS:
 path_dirs = t.split('/')
 try:
 build_top = rfind(path_dirs, 'build', -2)
@@ -326,21 +325,6 @@
 # tree (not specific to a particular build like ALPHA_SE)
 #
 
-# Variable validators & converters for global sticky variables
-def PathListMakeAbsolute(val):
-if not val:
-return val
-f = lambda p: abspath(expanduser(p))
-return ':'.join(map(f, val.split(':')))
-
-def PathListAllExist(key, val, env):
-if not val:
-return
-paths = val.split(':')
-for path in paths:
-if not isdir(path):
-raise SCons.Errors.UserError("Path does not exist: '%s'" % path)
-
 global_vars_file = joinpath(build_root, 'variables.global')
 
 global_vars = Variables(global_vars_file, args=ARGUMENTS)
@@ -351,8 +335,7 @@
 ('BATCH', 'Use batch pool for build and tests', False),
 ('BATCH_CMD', 'Batch pool submission command name', 'qdo'),
 ('M5_BUILD_CACHE', 'Cache built objects in this directory', False),
-('EXTRAS', 'Add extra directories to the compilation', '',
- PathListAllExist, PathListMakeAbsolute),
+('EXTRAS', 'Add extra directories to the compilation', '')
 )
 
 # Update main environment with values from ARGUMENTS & global_vars_file
@@ -363,10 +346,10 @@
 global_vars.Save(global_vars_file, main)
 
 # Parse EXTRAS variable to build list of all directories where we're
-# look for sources etc.  This list is exported as base_dir_list.
+# look for sources etc.  This list is exported as extras_dir_list.
 base_dir = main.srcdir.abspath
 if main['EXTRAS']:
-extras_dir_list = main['EXTRAS'].split(':')
+extras_dir_list = makePathListAbsolute(main['EXTRAS'].split(':'))
 else:
 extras_dir_list = []
 
@@ -808,6 +791,9 @@
 # Walk the tree and execute all SConsopts scripts that wil add to the
 # above variables
 for bdir in [ base_dir ] + extras_dir_list:
+if not isdir(bdir):
+print "Error: directory '%s' does not exist" % bdir
+Exit(1)
 for root, dirs, files in os.walk(bdir):
 

[m5-dev] changeset in m5: scons: allow use of current builds as default b...

[Steve Reinhardt]

changeset 06f3a4cbd585 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=06f3a4cbd585
description:
scons: allow use of current builds as default build settings
Currently the --default= option only looks at the predefined
build configs (in m5/build_opts), so you're limited to basing
a new build config off of those (ALPHA_SE, etc.).  If you've
already defined a non-standard build config and want to clone
it or tweak it, you have to start from scratch.  This patch
causes --default= to look first among the existing builds
(in build/variables) before looking in build_opts so you
can specify an existing non-standard build config as a
starting point for a new config.

diffstat:

 SConstruct |  20 +---
 1 files changed, 13 insertions(+), 7 deletions(-)

diffs (35 lines):

diff -r 66a3187a6714 -r 06f3a4cbd585 SConstruct
--- a/SConstructMon May 02 00:16:14 2011 -0400
+++ b/SConstructMon May 02 12:40:31 2011 -0700
@@ -967,18 +967,24 @@
 
 # Get default build variables from source tree.  Variables are
 # normally determined by name of $VARIANT_DIR, but can be
-# overriden by 'default=' arg on command line.
+# overridden by '--default=' arg on command line.
 default = GetOption('default')
-if not default:
-default = variant_dir
-default_vars_file = joinpath('build_opts', default)
-if isfile(default_vars_file):
+opts_dir = joinpath(main.root.abspath, 'build_opts')
+if default:
+default_vars_files = [joinpath(build_root, 'variables', default),
+  joinpath(opts_dir, default)]
+else:
+default_vars_files = [joinpath(opts_dir, variant_dir)]
+existing_files = filter(isfile, default_vars_files)
+if existing_files:
+default_vars_file = existing_files[0]
 sticky_vars.files.append(default_vars_file)
 print "Variables file %s not found,\n  using defaults in %s" \
   % (current_vars_file, default_vars_file)
 else:
-print "Error: cannot find variables file %s or %s" \
-  % (current_vars_file, default_vars_file)
+print "Error: cannot find variables file %s or " \
+  "default file(s) %s" \
+  % (current_vars_file, ' or '.join(default_vars_files))
 Exit(1)
 
 # Apply current variable settings to env
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Review Request: SConstruct: automatically update .hg/hgrc with style hooks

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/668/
---

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

SConstruct: automatically update .hg/hgrc with style hooks

Seems easier than pestering people about it.
Note also that path is now absolute, so you don't get errors
when invoking hg from subdirectories.


Diffs
-

  SConstruct 66a3187a6714 

Diff: http://reviews.m5sim.org/r/668/diff


Testing
---


Thanks,

Steve

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] src/dest detection in the ISA descriptions

[Steve Reinhardt]

On Wed, Apr 27, 2011 at 10:14 AM, Gabe Black  wrote:

> On 04/27/11 08:02, Steve Reinhardt wrote:
> > On Wed, Apr 27, 2011 at 1:21 AM, Gabe Black 
> wrote:
> >
> >>> Perhaps the heuristics could simply be extended to deal with
> >>> structure field accesses... if the thing after the symbol is a ".",
> then
> >> you
> >>> look past that to see if there's an equals sign or not, and behave
> >>> appropriately.
> >> "appropriately" isn't clearly defined, really. [...]
> >>
> > "appropriately" was just a placeholder for "stuff I'm sure we can figure
> out
> > later". [...]
>
> My point is that it -is- too complicated to try to figure out if you
> overwrite an entire dest or just part of it. That's why you need to be
> able to say which it is by hand.
>

That's fine... having a system that does the right thing automatically most
of the time (like it does now, ideally with extensions to avoid awkward
workarounds and increase the fraction of time it's automatically right)
coupled with a way to efficiently do manual overrides when it's not right
seems like a fine approach to me.  Extending the heuristics as I said could
still be a part of avoiding workarounds and increasing the number of cases
where it does the right thing automatically.


> >> My idea is to be able to inherit from the standard op types like
> >> IntRegOperand and allow them to install more than one index and/or
> >> change how they're declared, read, and written. So say for example you
> >> had a 128 bit wide SIMD instruction operating on four floating point
> >> registers which are normally accessible as 32 bit, indexed through some
> >> strange scheme. You could say that operand generates 4 indices,
> >> determined by whatever weird formula or just sequentially. Then you
> >> could define a structure which was the union of an array of 4 32 bit
> >> ints and 4 floats or 2 doubles, or 16 bytes, or ... The constructor
> >> could take the return of reading the 4 indices with
> >> readFloatRegOperandBits to fill it's main array, and then the
> >> instruction could access whatever other representation it needed. The
> >> other direction would work as well. Then, assuming we get this
> >> source/dest thing straightened out, you could have an instruction that,
> >> say, did parallel byte adds defined like this:
> >>
> >> for (int i = 0; i < 7; i++)
> >>Dest.bytes[i] = Source1.bytes[i] + Source2.bytes[i];
> >>
> >> And all the other goop would figure itself out. If some other scheme was
> >> needed (runtime selectable width, selectable float vs. int, etc) then
> >> the IntRegOperand subclass combined with the composite operand type
> >> could have the smarts to do the right thing all tidily hidden away after
> >> it was set up. Note that Dest is just a dest, even though it uses a
> >> structure field.
> >>
> > I'm not sure how you're envisioning this will work... are you assuming
> > there's a full C++ parser like gcc-xml?  How would you know that in this
> > case you're overwriting all of Dest, but if the upper bound of the loop
> was
> > 6 and not 7 then it would be just a partial overwrite?  That's a level of
> > compiler analysis I *really* don't want to get into.  (Yes, it is
> > theoretically feasible and it would be pretty slick, but I'm sure there
> are
> > many other things that would have a greater ROI than that.)
> >
> > Could we do this with C++ operator overloading?  Seems like you could
> just
> > say "Dest = Source1 + Source2;" which would be obvious to our code
> parser,
> > and then have C++ do the magic.  The type extensions could be used as
> casts
> > to make this more flexible, e.g., "Dest = Source1@bv + Source2@bv" could
> do
> > byte-by-byte adds while "Source1@fv + Source2@fv" could do it by 32-bit
> > floats, or something like that.
>
> The only tricky thing there is determining what is a source and what is
> a dest, although it highlights how hard that can be.


I still don't really get your proposal, in terms of how the code sample you
have parsed above would get turned into a concrete set of source and dest
operand indices.


> Operator
> overloading seems a little too magical to me, and it wouldn't be obvious
> to somebody looking at the code what's going on. I also don't like
> having to engineer all the possible operations ahead of time and/or
> define a type that does e

Re: [m5-dev] src/dest detection in the ISA descriptions

[Steve Reinhardt]

On Wed, Apr 27, 2011 at 1:21 AM, Gabe Black  wrote:

> The XML thing is an interesting possibility, and it avoids having to
> make a whole new thing that understands C++. It would still mean we'd
> have to make a whole new thing that understands XML (which is much
> easier), but then there's the chicken and egg issue you mention. I don't
> think we do anything sneaky with deciding what header files to include,
> though, and we could probably tell the tool about the operands somehow,
> maybe by making them globals that get thrown away later. The current
> approach is far from 100% correct in that sense, so if we're a little
> fuzzy around the edges that should be ok.
>

My concern is that parsing C is a much more complex and strictly defined
thing than what we currently do... the current approach degrades
"gracefully" in the sense that it might not get the results you want, but it
never fails completely.  In contrast, if there's a situation where X is a
type but gcc thinks it's a variable (or vice versa), the parsing could break
down pretty completely.


>  > This is why the ad hoc yet (as you say) surprisingly effective
> heuristics
> > were used.  Perhaps the heuristics could simply be extended to deal with
> > structure field accesses... if the thing after the symbol is a ".", then
> you
> > look past that to see if there's an equals sign or not, and behave
> > appropriately.
>
> "appropriately" isn't clearly defined, really. There's an example in ARM
> where there's a bit broken out of a status register which is set if some
> condition happens. If you set it, you're really setting the whole thing
> even though it looks like you're just setting the bitfield. Treating
> that operand as a source isn't necessary in that case even though it
> would normally be.
>

"appropriately" was just a placeholder for "stuff I'm sure we can figure out
later".  Maybe there are two classes of fields, one that is a partial field
(so that writing it means you have a source and a dest) and one that is a
"complete" field (so that writing it implies just a dest, not a source).  If
you want to automatically detect that you've written a set of partial fields
that covers the entire structure and thus you don't need to consider it a
source, I think you're asking too much... that's a place where either a
manual override or just using the current approach and adding an extra
useless assignment to the entire dest would be best, I think.


> The manual override would get us there, but one thing I worry about is
> that if BitUnions or composite operands are used often, you might need
> to have them all over the place. Some sort of inheritance or being able
> to associate the overrides with a blob of code once that gets reused a
> lot (like setting condition codes, checking predicates, etc.) would help.
>

Those all sound reasonable to me.


> My idea is to be able to inherit from the standard op types like
> IntRegOperand and allow them to install more than one index and/or
> change how they're declared, read, and written. So say for example you
> had a 128 bit wide SIMD instruction operating on four floating point
> registers which are normally accessible as 32 bit, indexed through some
> strange scheme. You could say that operand generates 4 indices,
> determined by whatever weird formula or just sequentially. Then you
> could define a structure which was the union of an array of 4 32 bit
> ints and 4 floats or 2 doubles, or 16 bytes, or ... The constructor
> could take the return of reading the 4 indices with
> readFloatRegOperandBits to fill it's main array, and then the
> instruction could access whatever other representation it needed. The
> other direction would work as well. Then, assuming we get this
> source/dest thing straightened out, you could have an instruction that,
> say, did parallel byte adds defined like this:
>
> for (int i = 0; i < 7; i++)
>Dest.bytes[i] = Source1.bytes[i] + Source2.bytes[i];
>
> And all the other goop would figure itself out. If some other scheme was
> needed (runtime selectable width, selectable float vs. int, etc) then
> the IntRegOperand subclass combined with the composite operand type
> could have the smarts to do the right thing all tidily hidden away after
> it was set up. Note that Dest is just a dest, even though it uses a
> structure field.
>

I'm not sure how you're envisioning this will work... are you assuming
there's a full C++ parser like gcc-xml?  How would you know that in this
case you're overwriting all of Dest, but if the upper bound of the loop was
6 and not 7 then it would be just a partial overwrite?  That's a level of
compiler analysis I *really* don't want to get into.  (Yes, it is
theoretically feasible and it would be pretty slick, but I'm sure there are
many other things that would have a greater ROI than that.)

Could we do this with C++ operator overloading?  Seems like you could just
say "Dest = Source1 + Source2;" which would be obvious to our code parser,
and 

Re: [m5-dev] src/dest detection in the ISA descriptions

[Steve Reinhardt]

On Tue, Apr 26, 2011 at 9:45 PM, Gabriel Michael Black <
gbl...@eecs.umich.edu> wrote:

> People not working with the ISA parser can stop here.
>

Here I am :-).


> Before I mentioned using @, and I have a patch that makes that change in
> the parser and in all the ISA descs. It was less painful than you may
> assume.
>

I haven't looked at your patches yet... is the main point of switching from
. to @ to allow structure field accesses?


> I don't think there's a good way to figure out whether operands are a
> source and/or destination in a setup like this. In the past, I've tried to
> come up with a scheme that would force gcc/C++ to figure it out for us, but
> no matter how clever/depraved I allow my solutions to be, they still don't
> work. It might be possible to use a fancy compiler system like LLVM to build
> a new tool to do it, but that isn't a near term solution (although I
> wouldn't rule it out entirely).


One potentially interesting angle to pursue is gcc-xml... I don't know much
about it except that, as the name implies, it generates an xml
representation rather than machine code, which should be reasonably
analyzable.  I bet it's still hairy, but it might be the least hairy
alternative.

I expect the big problem with most of these approaches is that the final
compilation context for most of the C snippets is not known at the time the
code is parsed by the parser for operands etc.  Thus there will be symbols
that you can't disambiguate as types etc. because the proper include files
haven't been included (which could be undecidable if the proper include
files are also being generated in the same isa_parser run, so maybe they
don't even exist yet at the point where you need them).

This is why the ad hoc yet (as you say) surprisingly effective heuristics
were used.  Perhaps the heuristics could simply be extended to deal with
structure field accesses... if the thing after the symbol is a ".", then you
look past that to see if there's an equals sign or not, and behave
appropriately.



> Another solution might be to explicitly list source and destination
> operands like inline assembly blocks.


I definitely don't like this idea.  One of the main motivations for the
current implementation was the old SimpleScalar ISA definition macros, which
required explicit listing of operands... and which as a result had subtle
bugs that went undetected for years where particular dependencies weren't
being enforced because someone left an operand out of the list.

As you say, the current approach "works remarkably well most of the time",
so while I'm all for improving it and addressing its shortcomings, I see no
need to throw it out and replace it with something that's more manual
effort, less concise, and more error prone.

An extension that allows you to manually override what the parser finds. to
deal with those cases where it's getting confused, would be fine though...
basically sort of what you've been doing already, except that instead of
"overriding" its heuristics by coming up with awkward code sequences that
trick it into doing the right thing, you have a more blunt and effective
path to do that.  That's probably not as big a step forward as you want, but
to me it's preferable to just switching to a fully manual explicit scheme.


> Any ideas how we can get this to work better? This is a big stumbling block
> for this sort of set up, and this will lead to operand types which can be
> automatically composited from component values instead of having to have
> hacks that, for instance, retrofit/filter code blobs to build double
> precision floating point values from single precision ones behind the
> scenes, or to merge control and condition code bits everywhere the whole
> control register is needed, etc. I expect that last part to be important for
> x86 to get the control flags dependency issues fixed in O3.
>

Would it help to have a way to explicitly declare compound operand types so
that the parser can treat them intelligently?  Maybe it could be as simple
as declaring "operand foo has fields a, b, and c" and having that
autmoatically turn into separate internal entries for foo, foo.a, foo.b, and
foo.c in the operand-matching code, which are linked together so that the
right inferences are made when any of them are found.

Just a thought...

Steve
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Defining Miss Latencies in Ruby

[Steve Reinhardt]

I don't have a strong opinion, and I'm not 100% sure I'm even following this
correctly (I just skimmed the thread), but I think "access latency" (or
"effective" or "average" access latency) would be the term that's most
self-explanatory.  To me "request latency" is a little ambiguous, since I
believe you're really talking about request-to-response latency, so singling
out requests is only one side of it.  Calling it "response latency" would
also be pretty descriptive (more so that "request latency" IMO).  Though any
of these would be an improvement, since as Korey pointed out "miss latency"
is downright misleading...

Steve

On Sat, Apr 23, 2011 at 5:13 AM, Korey Sewell  wrote:

> I'll post a reviewboard patch for the name change soon.
>
> On Thu, Apr 21, 2011 at 11:24 AM, Beckmann, Brad  >wrote:
>
> > Ha...those latency values have been referred to as miss latencies for so
> > long that I failed to realize that calling them miss latencies would be
> > confusing.  There is a lot of old history there, more than I care to go
> > into.  If you want to change the name to request latency, that is fine by
> > me.
> >
> > Brad
> >
> >
> > > -Original Message-
> > > From: m5-dev-boun...@m5sim.org [mailto:m5-dev-boun...@m5sim.org]
> > > On Behalf Of Korey Sewell
> > > Sent: Wednesday, April 20, 2011 9:24 PM
> > > To: M5 Developer List
> > > Subject: Re: [m5-dev] Defining Miss Latencies in Ruby
> > >
> > > Hi Brad,
> > > Don't you think this is a misnomer and instead should be called
> > > "request_latency:" , "request_latency_IFETCH", and etc.? I think this
> is
> > what
> > > is really being measuring anyway: the request latency from when a
> request
> > > leaves the cpu sequencer and then eventually finishes.
> > >
> > > But if you are going to say a stat is "miss latencies" and it
> encompasses
> > both
> > > hits and misses, I would think that's a bit paradoxical. Do you agree
> > there or
> > > do you think "miss latencies" fundamentally implies hit latencies as
> > well?
> > >
> > > (As far as tracking miss latencies based off machine type, thanks for
> the
> > tip.
> > > In the short term, since I just care about L1 hits/misses, I think I'll
> > be able to
> > > just say "if miss_latency > L1_latency" in my local tree)
> > >
> > > On Wed, Apr 20, 2011 at 11:37 PM, Beckmann, Brad
> > > wrote:
> > >
> > > > Sure, it is recording all miss latencies, including L1 cache hits.
> > > > And yes, those L1 hits will show up in the first bucket.  However, I
> > > > don't see how that is a bug.  If you don't want to include L1 hits in
> > > > the histogram, then look how the MOESI_hammer protocol tracks
> separate
> > > > miss latencies depending on the responding machine type.
> > > >
> > > > Brad
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: m5-dev-boun...@m5sim.org [mailto:m5-dev-
> > > boun...@m5sim.org] On
> > > > > Behalf Of Korey Sewell
> > > > > Sent: Wednesday, April 20, 2011 7:20 PM
> > > > > To: M5 Developer List
> > > > > Subject: Re: [m5-dev] Defining Miss Latencies in Ruby
> > > > >
> > > > > (comments inline)
> > > > >
> > > > > > I'm confused.  The miss_latency calculated by the sequencer is
> the
> > > > > miss
> > > > > > latency of the particular request, not just L1 cache hits.
> > > > > >
> > > > > I think I understand that, but even if it's just L1 hits, let's say
> > > > > that the
> > > > > L1 latency is 1 and you are running a workload with a high hit rate
> > > > > in the L1s. Then doesnt the code then continuously record that L1
> > > > > hit in the 1st histogram bucket? This would definitely be the case
> > > > > for L1 latencies of the more than 1, since it's hardcoded to record
> > > > > everything of a latency > 0 (basically all requests), right?
> > > > >
> > > > >
> > > > > >
> > > > > > If you're seeing a bunch of minimum latency requests, I suspect
> > > > > something
> > > > > > else is wrong.
> > > > >
> > > > >  For instance, is "issued_time" a cycle value or a tick value?
> > > > > >
> > > > > The issued_time is the cycles, as it is set in the makeRequest(),
> > > > > Sequencer function when a new Request is built.
> > > > > --
> > > > > - Korey
> > > > > ___
> > > > > m5-dev mailing list
> > > > > m5-dev@m5sim.org
> > > > > http://m5sim.org/mailman/listinfo/m5-dev
> > > >
> > > >
> > > > ___
> > > > m5-dev mailing list
> > > > m5-dev@m5sim.org
> > > > http://m5sim.org/mailman/listinfo/m5-dev
> > > >
> > >
> > >
> > >
> > > --
> > > - Korey
> > > ___
> > > m5-dev mailing list
> > > m5-dev@m5sim.org
> > > http://m5sim.org/mailman/listinfo/m5-dev
> >
> >
> > ___
> > m5-dev mailing list
> > m5-dev@m5sim.org
> > http://m5sim.org/mailman/listinfo/m5-dev
> >
>
>
>
> --
> - Korey
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.o

Re: [m5-dev] what scons can do

[Steve Reinhardt]

On Thu, Apr 21, 2011 at 4:43 PM, nathan binkert  wrote:

> > Are you sure?  Running SLICC every time just to get a (typically
> unchanging)
> > list of files is not exactly instantaneous, and the names of the SLICC
> > output files hardly ever change.  What about this approach:
> It is pretty fast (and this includes starting the python interpreter)
> 0.48s user 0.03s system 99% cpu 0.514 total
>
> >[...]
>
> I think you're pre-optimizing.  How long does the ISA parser actually
> take to parse the files?
>

Maybe so... I think there's a subconscious impression that it takes a while
because there's a phase in the build that takes a noticeable amount of time
and that's all the output you see.  If in fact that delay is 10% running
SLICC and 90% scons doing other stuff silently then I agree it's not such a
big deal.

One psychological solution would be to reduce the amount of output from
SLICC, making it a less obvious target for annoyance when you're waiting for
a build...

Steve
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] what scons can do

[Steve Reinhardt]

On Thu, Apr 21, 2011 at 2:11 PM, nathan binkert  wrote:

> Running SCons twice in the way you suggest would be FAR slower than
> just running SLICC and the ISA parser twice the way we do.
>
Are you sure?  Running SLICC every time just to get a (typically unchanging)
list of files is not exactly instantaneous, and the names of the SLICC
output files hardly ever change.  What about this approach:

1. store the list of output files
2. scons builds a dependence graph based on that stored list
3. when the SLICC files change and actually need to be re-parsed, we compare
the output file list generated as a side effect with the stored list
4. if those lists differ, scons blows itself away and starts over
5. if not, we can get by without parsing the SLICC files at all in the
common case where you're rebuilding the binary and none of the SLICC inputs
have changed

Yes, step 4 is ugly and expensive, but the key is that it should hardly ever
happen.  "Make the common case fast", and all that.  If we commit the output
file lists to hg, it wouldn't even need to happen on a clean build.

Just brainstorming...

Steve
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Introducing...InOrder AlphaFS!

[Steve Reinhardt]

Way to go, Korey!

On Thu, Apr 21, 2011 at 1:11 AM, Gabe Black  wrote:
> Tada! Congratulations.
>
> Gabe
>
> On 4/21/2011 12:12 AM, Korey Sewell wrote:
>>
>> For 1-4 wide, InOrder Cores running in ALPHA FS mode... Observe terminal
>> output:
>> "M5 console: m5AlphaAccess @ 0xFD02
>> Got Configuration 623
>> memsize 800 pages 4000
>> First free page after ROM 0xFC018000
>> HWRPB 0xFC018000 l1pt 0xFC04 l2pt 0xFC042000
>> l3pt_rpb 0xFC044000 l3pt_kernel 0xFC048000 l2reserv
>> 0xFC046000
>> kstart = 0xFC31, kend = 0xFC855898, kentry =
>> 0xFC31, numCPUs = 0x1
>> CPU Clock at 2000 MHz IntrClockFrequency=1024
>> Booting with 1 processor(s)
>> ...
>> Linux version 2.6.13 (h...@zed.eecs.umich.edu) (gcc version 3.4.3) #1 SMP
>> Sun Oct 8 ..
>> Brought up 1 CPUs
>> ...
>> init started:  BusyBox v1.1.0 (2007.03.04-01:07+) multi-call binary
>> mounting filesystems...
>> EXT2-fs warning: checktime reached, running e2fsck is recommended
>> loading script...
>> "*
>> **Two Words: "Yes Sir!"
>>
>> *More Than Two Words: InOrder is booting Linux!!!
>>
>> Getting to this point took about>30 patches, so I have a lot of
>> patch-cleaning work to do before I can place it in the repo.
>>
>> Also, just because it boots doesn't mean I've tested it with any kind of
>> real workload suite (e.g. SPEC or Parsec).  I don't have the bandwidth to
>> complete stress test through all the benchmarks and test but if someone
>> tries and something goes wrong, please post to the mailing list and I'll
>> do
>> my best to help you debug.
>>
>> Either way, I think this is a good milestone and I'll be adding the
>> patches
>> and the regression test for ALPHA_FS/10.linux_boot/InOrder-CPU within the
>> next week (or two) and then eventually updating more things in our
>> handy-dandy M5 status matrix:
>> http://m5sim.org/wiki/index.php/Status_Matrix
>>
>> The next big thing I want to get in InOrder before the "gem5" official
>> merge
>> is complete is timing TLB translation and in the longer term microcode
>> support. I can't say exactly when those will be added (again, not enough
>> time for me to completely focus on this), but if anyone wants to try I
>> have
>> a good idea of what needs to be done and can assist.
>>
>>
>
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Review Request: util/regress: make default action a more thorough regression

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/649/
---

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

util/regress: make default action a more thorough regression

Changed the --variants option to --test-variants and added a new
--compile-variants option for variants that are only compiled
(not tested).  The former still defaults to 'opt' and the latter
defaults to 'debug,fast'.

Also changed the behavior when no tests are specified from just
compiling to running the 'quick' tests.

As a result, a plain 'util/regress' invocation will now compile
(but not test) the debug and fast builds, and compile and run the
quick regressions on the opt build.  This should be the default
set of tests that are run before committing.  Since the nightly
regressions use this same script, this will also be the new
nightly regression behavior.

Test-only regressions can still be done by setting --compile=''.
Compile-only regressions can be done by setting --test=''.


Diffs
-

  util/regress a9d06c894afe 

Diff: http://reviews.m5sim.org/r/649/diff


Testing
---


Thanks,

Steve

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] changeset in m5: scons: Allow the build directory live under an ...

[Steve Reinhardt]

Yes, you are right and that is intentional.

Some of us discussed this (perhaps off-list), and Brad's earlier
version had another test similar to what you're proposing.  However,
you could have multiple build directories under your EXTRAS dir (e.g.,
extras/x86-64/build, extras/x86-32/build, and extras/arm/build) and
you wouldn't want to recurse down any of those since they would all
have undesirable SConscript symlinks, even though only one of them is
the one you're using.  So it seems there's no simple way to know if a
directory called 'build' should be traversed or not, but overall it
seemed like the odds of another random directory called 'build' being
a build directory was much much higher than the odds of it being
filled with source you really needed to compile, and as a bonus it
made the code simpler, so we went with the current version.

Plus there's always a workaround of "don't name any of your source
directories 'build'", which doesn't seem terribly onerous.

Steve

On Wed, Apr 20, 2011 at 7:22 PM, Gabe Black  wrote:
> It's probably a bit late to bring this up, but won't this remove -all-
> directories called build, not just -the- build directory? There's an os
> function that checks if two directories are the same. We can use that to
> compare the actual build directory with whatever we're recursing down. That
> would even catch hard links, I think, although hard linking your build
> directory would be a little weird.
>
> Gabe
>
> On 4/20/2011 11:20 AM, Brad Danofsky wrote:
>>
>> changeset f52ece27e20d in /z/repo/m5
>> details: http://repo.m5sim.org/m5?cmd=changeset;node=f52ece27e20d
>> description:
>>        scons: Allow the build directory live under an EXTRAS directory
>>
>> diffstat:
>>
>>  src/SConscript |  4 
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diffs (14 lines):
>>
>> diff -r ee4e795343bf -r f52ece27e20d src/SConscript
>> --- a/src/SConscript    Tue Apr 19 18:45:23 2011 -0700
>> +++ b/src/SConscript    Wed Apr 20 11:14:51 2011 -0700
>> @@ -320,6 +320,10 @@
>>  for extra_dir in extras_dir_list:
>>      prefix_len = len(dirname(extra_dir)) + 1
>>      for root, dirs, files in os.walk(extra_dir, topdown=True):
>> +        # if build lives in the extras directory, don't walk down it
>> +        if 'build' in dirs:
>> +            dirs.remove('build')
>> +
>>          if 'SConscript' in files:
>>              build_dir = joinpath(env['BUILDDIR'], root[prefix_len:])
>>              SConscript(joinpath(root, 'SConscript'),
>> variant_dir=build_dir)
>> ___
>> m5-dev mailing list
>> m5-dev@m5sim.org
>> http://m5sim.org/mailman/listinfo/m5-dev
>>
>
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] changeset in m5: Change default regression build from 'fast' to ...

[Steve Reinhardt]

changeset 95b2bf400ee4 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=95b2bf400ee4
description:
Change default regression build from 'fast' to 'opt'

diffstat:

 util/regress |  2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diffs (12 lines):

diff -r 63e849f0f341 -r 95b2bf400ee4 util/regress
--- a/util/regress  Wed Apr 20 11:14:52 2011 -0700
+++ b/util/regress  Wed Apr 20 13:47:42 2011 -0700
@@ -52,7 +52,7 @@
'X86_SE,X86_FS,' \
'ARM_SE,ARM_FS',
help="comma-separated build targets to test (default: '%default')")
-add_option('--variants', dest='variants', default='fast',
+add_option('--variants', dest='variants', default='opt',
help="comma-separated build variants to test (default: '%default')")
 add_option('--scons-opts', dest='scons_opts', default='', metavar='OPTS',
help='scons options')
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] another compilation error in the tree (!!)

[Steve Reinhardt]

This has to do with DPRINTFs in Ruby code, so I think it's related to
Nate's recent changes.  Note that the regressions pass because they
run with m5.fast which compiles out DPRINTFs.  I'm pretty sure I've
said this before, but I think we should run regressions with opt and
not fast (at least some of the time).  Certainly people should run
regressions with opt and not just fast before they commit, since this
also gives the opportunity to catch run-time assertions.

Steve


% util/regress -k --variant=debug
[...]
 [ CXX] 
ALPHA_SE_MESI_CMP_directory/mem/protocol/L2Cache_L1Cache_request_type_to_event.cc
-> .do
build/ALPHA_SE_MESI_CMP_directory/mem/protocol/L2Cache_L1Cache_request_type_to_event.cc:
In member function 'L2Cache_Event
L2Cache_Controller::L2Cache_L1Cache_request_type_to_event(CoherenceRequestType,
Address, MachineID, L2Cache_Entry*)':
build/ALPHA_SE_MESI_CMP_directory/mem/protocol/L2Cache_L1Cache_request_type_to_event.cc:32:
error: 'RubySlicc' is not a member of 'Debug'
scons: *** 
[build/ALPHA_SE_MESI_CMP_directory/mem/protocol/L2Cache_L1Cache_request_type_to_event.do]
Error 1
 [ CXX] ALPHA_SE_MESI_CMP_directory/mem/protocol/L2Cache_addSharer.cc -> .do
build/ALPHA_SE_MESI_CMP_directory/mem/protocol/L2Cache_addSharer.cc:
In member function 'void
L2Cache_Controller::L2Cache_addSharer(Address, MachineID,
L2Cache_Entry*)':
build/ALPHA_SE_MESI_CMP_directory/mem/protocol/L2Cache_addSharer.cc:23:
error: 'RubySlicc' is not a member of 'Debug'
scons: *** [build/ALPHA_SE_MESI_CMP_directory/mem/protocol/L2Cache_addSharer.do]
Error 1
scons: `build/ALPHA_SE_MOESI_CMP_directory/m5.debug' is up to date.
 [ CXX] 
ALPHA_SE_MOESI_CMP_token/mem/protocol/L1Cache_averageLatencyEstimate.cc
-> .do
build/ALPHA_SE_MOESI_CMP_token/mem/protocol/L1Cache_averageLatencyEstimate.cc:
In member function 'int
L1Cache_Controller::L1Cache_averageLatencyEstimate()':
build/ALPHA_SE_MOESI_CMP_token/mem/protocol/L1Cache_averageLatencyEstimate.cc:9:
error: 'RubySlicc' is not a member of 'Debug'
scons: *** 
[build/ALPHA_SE_MOESI_CMP_token/mem/protocol/L1Cache_averageLatencyEstimate.do]
Error 1
 [ CXX] 
ALPHA_SE_MOESI_CMP_token/mem/protocol/L1Cache_updateAverageLatencyEstimate.cc
-> .do
build/ALPHA_SE_MOESI_CMP_token/mem/protocol/L1Cache_updateAverageLatencyEstimate.cc:
In member function 'void
L1Cache_Controller::L1Cache_updateAverageLatencyEstimate(int)':
build/ALPHA_SE_MOESI_CMP_token/mem/protocol/L1Cache_updateAverageLatencyEstimate.cc:9:
error: 'RubySlicc' is not a member of 'Debug'
scons: *** 
[build/ALPHA_SE_MOESI_CMP_token/mem/protocol/L1Cache_updateAverageLatencyEstimate.do]
Error 1
 [ CXX] 
ALPHA_SE_MOESI_CMP_token/mem/protocol/L2Cache_convertToGenericType.cc
-> .do
build/ALPHA_SE_MOESI_CMP_token/mem/protocol/L2Cache_convertToGenericType.cc:
In member function 'GenericRequestType
L2Cache_Controller::L2Cache_convertToGenericType(CoherenceRequestType)':
build/ALPHA_SE_MOESI_CMP_token/mem/protocol/L2Cache_convertToGenericType.cc:15:
error: 'RubySlicc' is not a member of 'Debug'
scons: *** 
[build/ALPHA_SE_MOESI_CMP_token/mem/protocol/L2Cache_convertToGenericType.do]
Error 1
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Stats Bug

[Steve Reinhardt]

I'd definitely like to see us focus our efforts on Ruby.  That said,
if you've got the code working, there's no reason not to use it.
Also, I'm guessing that a similar problem will exist in Ruby if the
Ruby cache ever extends its stats to be per context... I expect the
only reason Ruby doesn't have this problem is that the stats are not
segregated that way.  Someone speak up if I'm wrong about that please.

Steve

On Wed, Apr 20, 2011 at 10:04 AM, Korey Sewell  wrote:
>> I think Lisa's "pythonic fix" is the one where we use the _numCpus
>> value that's calculated entirely in python (including the +1 offset in
>> FS mode) and don't attempt the automatic C++ technique that Korey
>> proposed.
>
> Hey guys,
> with the new Ruby memory system coming, how much of the classic memory
> system support (or features) are we still going to be adding on?
>
> I ask because I was curious about this last night and  wrote a good chunk of
> the code for the "automatic C++ technique" (actually wasnt too hard to
> write). The problem Lisa posed of hierarchy confusion I approached and just
> now thought how to truly solve it. Basically, instead of passing a "sharer
> count" from upstream caches, pass a set of sharers and then you keep adding
> to that set as you combine results from multiple ports on a bus. I also
> coded it so that you only do this "auto-technique" if an original parameter
> wasn't set (so Lisa's way would still work).
>
> So I guess what I am asking is does anyone care about that feature of
> automatically determining the sharers OR since Ruby will be the preferred
> memory system that any feature (or complexity) I add to this would be
> unnecessary in the big picture?
>
> If it's the latter, I'm OK with that, I can just post what I have to the
> reviewboard at some point for someone who might care in the future (its at
> the back of a long patchqueue right now).
>
> If people do care or think it would be a cool add-on, I can wait until Lisa
> does her 1st pass post of this and then at that point match the variable
> names and post my "auto-solution".
>
> --
> - Korey
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: stats: rename stats so they can be used as python expressions

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/645/#review1138
---

Ship it!


I'm sure this will cause some people headaches if they have stat names 
hardcoded in some scripts, but it's unavoidable.

- Steve


On 2011-04-19 13:20:04, Nathan Binkert wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/645/
> ---
> 
> (Updated 2011-04-19 13:20:04)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> stats: rename stats so they can be used as python expressions
> 
> 
> Diffs
> -
> 
>   src/cpu/inorder/pipeline_stage.cc d8ec0a7b3f0c 
>   src/cpu/inorder/resource_pool.9stage.cc d8ec0a7b3f0c 
>   src/cpu/inorder/resource_pool.cc d8ec0a7b3f0c 
>   src/cpu/o3/commit_impl.hh d8ec0a7b3f0c 
>   src/cpu/o3/decode_impl.hh d8ec0a7b3f0c 
>   src/cpu/o3/iew_impl.hh d8ec0a7b3f0c 
>   src/cpu/o3/inst_queue_impl.hh d8ec0a7b3f0c 
>   src/cpu/o3/rename_impl.hh d8ec0a7b3f0c 
>   src/mem/cache/tags/base.cc d8ec0a7b3f0c 
>   src/sim/process.cc d8ec0a7b3f0c 
> 
> Diff: http://reviews.m5sim.org/r/645/diff
> 
> 
> Testing
> ---
> 
> I'm rerunning all tests right now and will commit the results so all the name 
> changes are updated.
> 
> 
> Thanks,
> 
> Nathan
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Stats Bug

[Steve Reinhardt]

I think Lisa's "pythonic fix" is the one where we use the _numCpus
value that's calculated entirely in python (including the +1 offset in
FS mode) and don't attempt the automatic C++ technique that Korey
proposed.  Yes, that still involves C++ changes, specifically making
everything in the C++ code just use the value passed in from python.
That value is currently called _numCpus.  IMO, _numCpus is not a good
name, and while we're at it we should change it to be more
descriptive, like _numSharers or _numSharingContexts.

Steve

On Tue, Apr 19, 2011 at 3:14 PM, Korey Sewell  wrote:
> I think "_numCpus+1" is the safe choiceLisa?
>
> On Tue, Apr 19, 2011 at 6:01 PM, nathan binkert  wrote:
>>> I think the immediate fix is the pythonic fix from a few msgs ago.  I
>>> believe that's quick and easy.  I think :).
>>
>> The fix has to be more than python.  I'm talking about the original
>> question.  The stats use three different lengths: "_numCpus + 1",
>> "_numCpus",
>> or "maxThreadsPerCPU".  Which one is correct?
>>
>> Are you healthy enough to fix this?
>>
>> Thanks,
>>
>>  Nate
>> ___
>> m5-dev mailing list
>> m5-dev@m5sim.org
>> http://m5sim.org/mailman/listinfo/m5-dev
>>
>
>
>
> --
> - Korey
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] what scons can do

[Steve Reinhardt]

On Tue, Apr 19, 2011 at 3:13 PM, Gabriel Michael Black
 wrote:
>>
>> Caching the list of generated SLICC files sounds like a good idea to
>> me.  I'm not sure this would require recursive scons invocations,
>> since we manage to build the list dynamically already without that.  I
>> wouldn't call it a cache of "dependency information" though, since
>> scons already has one of those; this is really just a cache of
>> generated filenames, right?
>
> How would you be able to do it all in one shot? The tricky part is that you
> actually have to build the .dep in the build phase, but you need it in the
> dependency tree generating phase. Since you can't go backwards like that in
> one invocation (as far as I know or can imagine) then you'd need to rounds.

OK, I see, maybe it is inherently another level beyond what we
currently do.  I'm not the scons expert...

> As far as what it would be caching I think it's largely a semantic
> difference. You could consider it a cache of generated files which are used
> to set up the dependencies.

It's just terminology... not that it's wildly inaccurate to call it
"dependency info", since it is info that is eventually used to
determine dependencies, just that there's already a "dependency
caching" feature in scons that's totally different
(http://www.scons.org/doc/2.0.1/HTML/scons-user.html#AEN1148), and in
general when the scons docs talk about dependencies it's "what are the
files that this file depends on" not "what are the files that depend
on this file".  Thus it would be less confusing if you avoided
referring to the info that you're discussing here as "dependency info"
and used a different term like "generated file info".  That's all.

Steve
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Stats Bug

[Steve Reinhardt]

Lisa should verify, but I think that's what _numCpus is.

On Tue, Apr 19, 2011 at 8:01 AM, Korey Sewell  wrote:
> Hey Steve,
> I dont disagree with your solution (at least in the interim), but
> wouldn't the right solution be to have the actual CPUs pass how many
> hardware threads is has to the caches?
>
> The actual "regStats" happens after all the CPUs are instantiated and
> ports are connected (right?), so it would seem that since different
> CPUs can have a different amount of threads per CPU, that the caches
> should just use the port interface to ask "how many threads?" .  Then,
> the caches down the hierarchy would just sum those thread counts up
> for each of their Stat vectors.
>
> I imagine a similar method to how the "M5 classic" figured out address
> ranges and snoop ports would work fine.
>
> Do people think that's fine or am I missing the point here?
>
> On Tue, Apr 19, 2011 at 10:42 AM, Steve Reinhardt  wrote:
>> Looks like it's Lisa's fault ;-)
>>
>> http://repo.m5sim.org/m5/diff/ab05e20dc4a7/src/mem/cache/base.cc
>>
>> I think Nate's point is that all the stats vector lengths should be
>> changed to _numCpus or _numCpus+1 instead of maxThreadsPerCpu to be
>> consistent.
>>
>> We should also either (1) always do _numCpus+1 even though the extra
>> "device" slot is unnecessary for SE mode or (2) have a single #ifdef
>> to set a local var to one or the other and use that consistently
>> rather than having #ifdefs all over the place.  I'd lean toward #2
>> just to keep the output a little cleaner in SE mode.
>>
>> Does that make sense, Lisa?
>>
>> Steve
>>
>> On Mon, Apr 18, 2011 at 3:58 PM, nathan binkert  wrote:
>>> Yes, but all arithmetic between vectors is elementwise, so they need
>>> to be the same length if used in a formula. Total miss latency needs
>>> to have the same vector length as total misses.
>>>
>>> Nate
>>>
>>> On Mon, Apr 18, 2011 at 2:09 PM, Lisa Hsu  wrote:
>>>> I'm not sure I understand what the problem is either.  Can different
>>>> VectorStats not have different lengths?
>>>>
>>>> Lisa
>>>>
>>>> On Mon, Apr 18, 2011 at 11:43 AM, Gabriel Michael Black <
>>>> gbl...@eecs.umich.edu> wrote:
>>>>
>>>>> My first reaction is "let's fix it", but I don't really understand the
>>>>> problem or the impact of changing things. Anything serious?
>>>>>
>>>>> Gabe
>>>>>
>>>>>
>>>>> Quoting nathan binkert :
>>>>>
>>>>>  I'm trying to get my python stats stuff committed and I found a bug in
>>>>>> the classic cache stats.  Look in src/mem/cache/base.cc.  The
>>>>>> VectorStats have several different lengths "_numCpus + 1", "_numCpus",
>>>>>> or "maxThreadsPerCPU".
>>>>>>
>>>>>> The fact that this works in the current stats package is lucky.  I can
>>>>>> be bug compatible, but I think we should fix this instead.
>>>>>>
>>>>>>  Nate
>>>>>> ___
>>>>>> m5-dev mailing list
>>>>>> m5-dev@m5sim.org
>>>>>> http://m5sim.org/mailman/listinfo/m5-dev
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> ___
>>>>> m5-dev mailing list
>>>>> m5-dev@m5sim.org
>>>>> http://m5sim.org/mailman/listinfo/m5-dev
>>>>>
>>>>>
>>>> ___
>>>> m5-dev mailing list
>>>> m5-dev@m5sim.org
>>>> http://m5sim.org/mailman/listinfo/m5-dev
>>>>
>>> ___
>>> m5-dev mailing list
>>> m5-dev@m5sim.org
>>> http://m5sim.org/mailman/listinfo/m5-dev
>>>
>> ___
>> m5-dev mailing list
>> m5-dev@m5sim.org
>> http://m5sim.org/mailman/listinfo/m5-dev
>>
>
>
>
> --
> - Korey
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Stats Bug

[Steve Reinhardt]

Looks like it's Lisa's fault ;-)

http://repo.m5sim.org/m5/diff/ab05e20dc4a7/src/mem/cache/base.cc

I think Nate's point is that all the stats vector lengths should be
changed to _numCpus or _numCpus+1 instead of maxThreadsPerCpu to be
consistent.

We should also either (1) always do _numCpus+1 even though the extra
"device" slot is unnecessary for SE mode or (2) have a single #ifdef
to set a local var to one or the other and use that consistently
rather than having #ifdefs all over the place.  I'd lean toward #2
just to keep the output a little cleaner in SE mode.

Does that make sense, Lisa?

Steve

On Mon, Apr 18, 2011 at 3:58 PM, nathan binkert  wrote:
> Yes, but all arithmetic between vectors is elementwise, so they need
> to be the same length if used in a formula. Total miss latency needs
> to have the same vector length as total misses.
>
> Nate
>
> On Mon, Apr 18, 2011 at 2:09 PM, Lisa Hsu  wrote:
>> I'm not sure I understand what the problem is either.  Can different
>> VectorStats not have different lengths?
>>
>> Lisa
>>
>> On Mon, Apr 18, 2011 at 11:43 AM, Gabriel Michael Black <
>> gbl...@eecs.umich.edu> wrote:
>>
>>> My first reaction is "let's fix it", but I don't really understand the
>>> problem or the impact of changing things. Anything serious?
>>>
>>> Gabe
>>>
>>>
>>> Quoting nathan binkert :
>>>
>>>  I'm trying to get my python stats stuff committed and I found a bug in
 the classic cache stats.  Look in src/mem/cache/base.cc.  The
 VectorStats have several different lengths "_numCpus + 1", "_numCpus",
 or "maxThreadsPerCPU".

 The fact that this works in the current stats package is lucky.  I can
 be bug compatible, but I think we should fix this instead.

  Nate
 ___
 m5-dev mailing list
 m5-dev@m5sim.org
 http://m5sim.org/mailman/listinfo/m5-dev


>>>
>>>
>>> ___
>>> m5-dev mailing list
>>> m5-dev@m5sim.org
>>> http://m5sim.org/mailman/listinfo/m5-dev
>>>
>>>
>> ___
>> m5-dev mailing list
>> m5-dev@m5sim.org
>> http://m5sim.org/mailman/listinfo/m5-dev
>>
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] what scons can do

[Steve Reinhardt]

On Tue, Apr 19, 2011 at 3:00 AM, Gabe Black  wrote:
> I was looking at some of the stuff in util, and it occurred to me that
> the m5 utility program is cross compiled using different Makefiles for
> different architectures. Statetrace used to be like that (sort of), but
> recently I converted it over to scons and set up some configuration
> variables that made it easier to work with. It would be nice to be able
> to share some of that with the m5 utility program, although I don't
> remember it being all that complicated.
>
> Anyway, it seems like it would be useful to be able to have multiple
> binaries that can be built by scons, specifically the utility stuff and
> unit tests. That way we could avoid having a hodge podge of small build
> systems which are either isolated or not in not quite the right ways.

If you're suggesting that the stuff in util get built by scons and put
the binaries somewhere like build/util, I think that's a great idea.
I don't know of the pros and cons of making it an "indepedent"
invocation of scons vs integrating it into our global scons
configuration, but whatever way we do it should avoid redundant scons
code for things like detecting your compiler version, which makes me
think the integrated approach might be better.  Either way might
require some refactoring between the SConstruct file and
src/SConscript, since it's not under src, but we should be able to
work that out.

> Also, I was thinking about how to handle the dependencies/generated
> files/custom language issue a little while back, and what I kept coming
> back to were schemes where scons would use a cache of dependency
> information which it would regenerate if any of the input files which
> determined outputs and/or dependencies changed. The problem is that
> scons would need to run once and possibly regenerate its cache, and then
> run again to actually run. Is this sort of multi-pass setup possible
> somehow without major hacks?
>
> To explain more what I'm getting, lets say you have input file foo.isa
> which, when processed, generates the files foo_exec.cc and bar_exec.cc.
> What would happen is that you'd have a file like foo.isa.dep which would
> describe what would happen and make that depend on foo.isa.
>
> When you run for the first time, scons would see that foo.isa.dep
> doesn't exist. During it's build phase, it would run foo.isa through the
> system and see that it generated foo_exec.cc and bar_exec.cc and put
> that into foo.isa.dep (as actual SConscript type code, or flat data,
> or...). When scons ran the second time, it would read in foo.isa.dep and
> extract the dependencies from it and build that into the graph. It
> wouldn't construct foo.isa.dep again since all its inputs were the same,
> and it would still capture all those dependencies. This time around, the
> larger binary would see that it depended on foo_exec.cc and bar_exec.cc
> and that those depend on foo.isa.dep (as a convenient aggregation point
> of all *.isa files involved). If foo.isa changed later, foo.isa.dep
> would be out of date and have to be regenerated, and then foo_exec.cc
> and bar_exec.cc, and then the main binary.
>
> The net effect of this is that the thing that processed the .isa would
> only be run when necessary. In our current setup, that would mean SLICC
> wouldn't have to be run for every build, only ones where the SLICC input
> files changed. The problem here is that scons would need to basically
> call a nested instance of itself on foo.isa.dep, let that build a dep
> tree and run the build phase, then process foo.isa.dep in the parent dep
> phase, and then run the parent build phase. It could literally just call
> scons from scons (though that seems like a major hack) or it could, if
> scons has a facility for it, do some sort of fancy multi-pass thing.
>
> This is sort of related to the first thing (additional targets) because
> the dependency cache files are sort of like independent targets with
> their own invocations of scons.

Caching the list of generated SLICC files sounds like a good idea to
me.  I'm not sure this would require recursive scons invocations,
since we manage to build the list dynamically already without that.  I
wouldn't call it a cache of "dependency information" though, since
scons already has one of those; this is really just a cache of
generated filenames, right?

> Also related to scons are those .pyc files that end up scattered around
> the source tree. I know I asked about those a long, long time ago, but
> why are they there? Why don't they end up in the build directories?

That's an artifact of including them in scons, where they're run in
place and not by m5.

Steve
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Bug in changeset 8225 or 8227

[Steve Reinhardt]

Yea, if you look at the mentioned line, it's "mode = 'r+' if inplace
else 'r'", and conditional assignments like this (basically python's
version of C's "?:") weren't added until 2.5.  Since we officially
only require python 2.4 (not 2.5), I'm happy to call this Nate's
fault.  If you want to change the code to be 2.4 compatible you can,
or you can upgrade your python, or you can just roll back to an
earlier changeset in your local repo.

Steve

On Sat, Apr 16, 2011 at 12:10 PM, Nilay Vaish  wrote:
> On Sat, 16 Apr 2011, nathan binkert wrote:
>
>> What version of python are you using?  It could be that that syntax
>> wasn't available until 2.5 and you're using something older.  I can't
>> do anything about it right now because I'm about to leave on a hike.
>>
>>  Nate
>>
>>> Nate, it seems one of your checkins from yesterday has a bug. I receive
>>> the
>>> following message on executing any merculrial command.
>>>
>>> *** failed to import extension style from ./util/style.py: invalid syntax
>>> (file_types.py, line 143)
>>>
>>>
>>> --
>>> Nilay
>
> I am using python version 2.4.3.
>
> Nilay
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Ruby: Add support for functional accesses

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/611/#review1130
---


Thanks for pointing these changes out.  I didn't look at the Ruby stuff since 
it looks like Brad has beaten you up plenty on that.

I think it's fine to check this in initially without any support for handling 
errors outside of the tester.  I checked and there aren't many unique calls to 
sendFunctional; most of the uses of functional accesses go through 
Port::readBlob() and Port::writeBlob().  Adding code to check the return cmd 
value at each call site and call warn() if it fails would be the next step in 
my opinion.




configs/example/ruby_mem_test.py


You probably don't want to commit this value, do you?  If you want to 
hardwire a number, I'd pick something more reasonable (somewhere between 10 and 
25, just guessing).



src/mem/packet.hh


I don't see much value in having separate error return codes for reads and 
writes.  I'd use a single code that ends in Error (like FunctionalAccessError), 
and put it up a couple of lines with the other Error codes.



src/mem/packet.hh


Way too much code duplication here :-).  I think this works, correct?

void
makeFunctionalResponse(bool success)
{
makeResponse();
if (!success) {
cmd = MemCmd::FunctionalAccessError;
}
}



- Steve


On 2011-04-13 14:29:01, Nilay Vaish wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/611/
> ---
> 
> (Updated 2011-04-13 14:29:01)
> 
> 
> Review request for Default.
> 
> 
> Summary
> ---
> 
> Ruby: Add support for functional accesses
> This patch is meant for implementing functional access support in Ruby.
> Currently, the patch does not functional accesses for the PioPort.
> 
> 
> Diffs
> -
> 
>   configs/example/ruby_mem_test.py 8b5f900233ee 
>   configs/ruby/MESI_CMP_directory.py 8b5f900233ee 
>   configs/ruby/Ruby.py 8b5f900233ee 
>   src/cpu/testers/memtest/memtest.cc 8b5f900233ee 
>   src/mem/packet.hh 8b5f900233ee 
>   src/mem/packet.cc 8b5f900233ee 
>   src/mem/protocol/MESI_CMP_directory-L1cache.sm 8b5f900233ee 
>   src/mem/protocol/MESI_CMP_directory-L2cache.sm 8b5f900233ee 
>   src/mem/protocol/MESI_CMP_directory-dir.sm 8b5f900233ee 
>   src/mem/protocol/RubySlicc_Types.sm 8b5f900233ee 
>   src/mem/ruby/network/Network.cc 8b5f900233ee 
>   src/mem/ruby/network/Network.py 8b5f900233ee 
>   src/mem/ruby/profiler/Profiler.cc 8b5f900233ee 
>   src/mem/ruby/profiler/Profiler.py 8b5f900233ee 
>   src/mem/ruby/recorder/Tracer.cc 8b5f900233ee 
>   src/mem/ruby/recorder/Tracer.py 8b5f900233ee 
>   src/mem/ruby/system/AbstractMemory.hh PRE-CREATION 
>   src/mem/ruby/system/AbstractMemory.cc PRE-CREATION 
>   src/mem/ruby/system/AbstractMemory.py PRE-CREATION 
>   src/mem/ruby/system/Cache.py 8b5f900233ee 
>   src/mem/ruby/system/CacheMemory.hh 8b5f900233ee 
>   src/mem/ruby/system/CacheMemory.cc 8b5f900233ee 
>   src/mem/ruby/system/DirectoryMemory.hh 8b5f900233ee 
>   src/mem/ruby/system/DirectoryMemory.cc 8b5f900233ee 
>   src/mem/ruby/system/DirectoryMemory.py 8b5f900233ee 
>   src/mem/ruby/system/RubyPort.hh 8b5f900233ee 
>   src/mem/ruby/system/RubyPort.cc 8b5f900233ee 
>   src/mem/ruby/system/RubySystem.py 8b5f900233ee 
>   src/mem/ruby/system/SConscript 8b5f900233ee 
>   src/mem/ruby/system/Sequencer.cc 8b5f900233ee 
>   src/mem/ruby/system/Sequencer.py 8b5f900233ee 
>   src/mem/ruby/system/System.hh 8b5f900233ee 
>   src/mem/ruby/system/System.cc 8b5f900233ee 
>   src/mem/slicc/ast/MemberExprAST.py 8b5f900233ee 
> 
> Diff: http://reviews.m5sim.org/r/611/diff
> 
> 
> Testing
> ---
> 
> I have tested functional accesses with the ratio between functional
> and timing accesses for different ratios -- 100:0, 99:1, 90:1, 50:50,
> 10:90, 1:99. It is working in all the cases.
> 
> 
> Thanks,
> 
> Nilay
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] changeset in m5: includes: fix up code after sorting

[Steve Reinhardt]

On Fri, Apr 15, 2011 at 3:42 PM, nathan binkert  wrote:
> I noticed a clever way to fix this in the google style guide that we
> could adopt if we choose.
> http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Names_and_Order_of_Includes

Cute... seems like a nice little tweak.  I assume your script can do
this automatically?  :-)
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] changeset in m5: includes: fix up code after sorting

[Steve Reinhardt]

Are these just fixups where we were getting away with not including
needed files, and reordering the includes exposed that?  Or are there
other causes?  Superficially, the fact that a fix-up was needed after
running your script doesn't look good for your script... I'm assuming
it's a one-time thing but just want to verify that.

Steve

On Fri, Apr 15, 2011 at 3:31 PM, Nathan Binkert  wrote:
> changeset 845c8eb5ac49 in /z/repo/m5
> details: http://repo.m5sim.org/m5?cmd=changeset;node=845c8eb5ac49
> description:
>        includes: fix up code after sorting
>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Ruby patch touching classic memory system

[Steve Reinhardt]

Thanks for the heads up... I've been kind of swamped and am way behind
on code reviews.  I just took a look and I will have some comments for
you :-).

Steve

On Thu, Apr 14, 2011 at 9:30 AM, Nilay Vaish  wrote:
> Ali, Nate, Steve
>
> Can you go through this patch? I have made some changes to src/mem/packet.hh
> and src/mem/packet.cc files.
>
> http://reviews.m5sim.org/r/611/
>
>
> Thanks
> Nilay
>
>
> On Thu, 14 Apr 2011, Nilay Vaish wrote:
>
>>
>>
>>> On 2011-04-13 13:43:26, Gabe Black wrote:

 Please fix these style issues, including the ones in this file I haven't
 explicitly pointed out. You should be using M5 style generally, but
 especially when your in M5 code. Also, please be sure to point this out to
 one of the classic memory system experts (Nate, Steve, or Ali) and get them
 to sign off. They might not see that this change touches outside of Ruby.
>>
>> Gabe, I made the changes that you had pointed.
>>
>>
>> - Nilay
>>
>>
>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Cron /z/m5/regression/do-regression quick

[Steve Reinhardt]

We've had multiple discussions on this (search the list archives for
m5-stable and you should find them).  We had some debate about how
frequently m5-stable should be updated, and how long we want a
changeset to mature in m5 before we consider promoting it to
m5-stable, but I think we found some values everyone was content with
last time... something like every 6 months we'll update m5-stable to
the last working revision more than 1 month old, or something like
that.  (Or maybe it was 3/1, or 6/2, I forgot.)  But as Ali says, the
catch in automating this process is identifying the "last working
revision"... we could use the regression tests to help narrow that
down, but there are a lot of bugs that get pushed that aren't caught
by the regression tester, so I wouldn't want to rely solely on that.
If we had a better bug-tracking system so we could record facts like
"changeset Y fixes a bug introduced in changeset X" then we could
automatically exclude changesets between X and Y, but we don't have
that.

Steve

On Tue, Mar 29, 2011 at 6:38 PM, Ali Saidi  wrote:
> You could do that, but there is no guarentee you'd pick a non-broken version 
> to push. We wouldn't want to push anything from the last week with all the 
> compilation issues.
>
> Ali
>
> On Mar 29, 2011, at 6:19 PM, Korey Sewell wrote:
>
>>> I'd prefer to see us just start updating m5-stable more regularly so
>>> it can fulfill its original purpose.  We keep discussing this but
>>> never actually follow through.
>>
>> Is this any harder than just setting up a cronjob to push whatever is
>> in m5-dev to m5-stable once per month (?)
>>
>> - Korey
>> ___
>> m5-dev mailing list
>> m5-dev@m5sim.org
>> http://m5sim.org/mailman/listinfo/m5-dev
>>
>
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Add the user settable separator string for arrayed stats, default is standard ::

[Steve Reinhardt]

> On 2011-03-29 09:05:00, Gabe Black wrote:
> > Why do you need this functionality? Why isn't "::" sufficient? If it's 
> > because some other piece of software is expecting something else, I'd say 
> > either write a script that munges these stats into a more acceptable 
> > format, or adjust the other software. I'm not an expert on our stats stuff 
> > by any stretch, but I'd hate to see extra otherwise unnecessary 
> > functionality be added to deal with a point issue that only affects a few 
> > people.
> 
> brad danofsky wrote:
> You are correct that this may just be my problem.  I have a whole lot of 
> scripts that expect a different character (one used in a previous version of 
> m5 I think).  If it is objectionable that there be support for this then so 
> be it.

These are scripts that date back to when we used a different separator (for 
SimpleScalar compatibility I believe), so arguably it's just fixing something 
we broke in m5 a long time ago.  I discussed this with Brad, and we decided 
that in the long run the ideal thing would be to leverage the any-day-now 
stats-in-python functionality to have m5 spit out stats directly in the format 
that these scripts currently generate, making the scripts obsolete.  But in the 
meantime it's a much smaller change to tweak m5 than to edit this pile of 
scripts, and I don't think it's hurting anyone to put this in there.


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/610/#review1025
---


On 2011-03-29 09:43:16, brad danofsky wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/610/
> -------
> 
> (Updated 2011-03-29 09:43:16)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> Add the user settable separator string for arrayed stats, default is standard 
> ::
> 
> I updated the diff to fix Gabes issues and I learned how to spell.
> 
> 
> Diffs
> -
> 
>   src/base/statistics.hh d8587c913ccf 
>   src/base/statistics.cc d8587c913ccf 
>   src/base/stats/info.hh d8587c913ccf 
>   src/base/stats/text.cc d8587c913ccf 
> 
> Diff: http://reviews.m5sim.org/r/610/diff
> 
> 
> Testing
> ---
> 
> standard full regression
> 
> 
> Thanks,
> 
> brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Cron /z/m5/regression/do-regression quick

[Steve Reinhardt]

On Tue, Mar 29, 2011 at 10:54 AM, Gabe Black  wrote:
>
> That makes sense and was what I was hoping we could use m5-stable for,
> but we didn't end up doing that because we wanted m5-stable to be even
> more stable than that (although it tends to just be old, not necessarily
> stable). Maybe we need something in the middle that works like you're
> suggesting.

I'd prefer to see us just start updating m5-stable more regularly so
it can fulfill its original purpose.  We keep discussing this but
never actually follow through.

Steve
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Cron /z/m5/regression/do-regression quick

[Steve Reinhardt]

On Tue, Mar 29, 2011 at 10:34 AM, Korey Sewell  wrote:
> Good point about regression carefulness Gabe.
>
> Although I'm pretty sure I compiled MIPS before I committed this, I
> had forgot I touched other ISAs and obviously broke one of them.
> That's just an error on my part.
>
> This brings up a few issues:
> - Should the regressions be more resilient? In other words, if POWER
> doesnt compile shouldnt the regressions still *try* for whatever CPU
> model and ISA combinations it does compile for? (i'll add that to the
> regression wants page)

They already do... note that everything else passed.

> - It would be nice to somehow be able to localize the regression tests
> you need to run after making changes. There's about 5 ISAs (?), 4 CPU
> models , FS/SE mode, so if you make a particular change, sometimes its
> unwieldy (albeit still necessary) to run every single test. Is there
> an easy way to test just "what matters"?

There isn't (and it's already on the wish list), but it seems that's
actually part of the problem here: some things broke that you didn't
think you had affected.  OTOH, if you're talking about having the
system automatically detect what needs to be run, it already does that
(to some extent) by being built into scons... it will only compile the
binaries and run the tests that depend on files that have changed,
modulo scons bugs.

>
> - Lastly, An overkill thing would be to have a "buffer-repo" sitting
> between users' local repo and m5-dev. Then, nightly before the
> regressions, you could conceivably test to make sure that builds (or
> whatever other sanity check) and *then* check-in changesets to the
> dev-repo after the initial 1st pass. This would ensure that any pulls
> from m5-dev would always be compilable at the very least.

That's why we have m5-stable, basically.

Steve

>
>
> On Tue, Mar 29, 2011 at 1:21 PM, Gabe Black  wrote:
>> Compilation of POWER was broken by this change when the closing bracket
>> for a namespace was removed:
>>
>> changeset:   8181:f789b9aac5f4
>> user:        Korey Sewell 
>> date:        Sat Mar 26 09:23:52 2011 -0400
>> summary:     mips: cleanup ISA-specific code
>>
>> Compilation was unavoidably broken, and that means this code was not
>> compiled before being checked in. According to the logs, three of the
>> last four commits were to fix outdated regression output, broken
>> configuration scripts, and broken compilation, all of which could have
>> been detected before hand.
>>
>> We all have to be more careful. When was the last time someone could
>> download the dev repository and actually run everything successfully?
>> How many people downloaded a broken version of M5 in that time? As the
>> number of committers increases, it becomes more and more important to
>> push clean changes. If 100 people each break the simulator 1% of the
>> time, the simulator will be broken most of the time.
>>
>> It's also important to keep an eye on the regression output after you
>> push something and investigate any failures you see. I don't want to be
>> the regression police.
>>
>> Gabe
>>
>> On 03/29/11 03:50, Cron Daemon wrote:
>>> scons: *** [build/POWER_SE/kern/linux/linux.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/insts/branch.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/insts/mem.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/insts/integer.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/insts/floating.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/insts/condition.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/insts/static_inst.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/pagetable.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/tlb.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/utility.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/process.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/linux/process.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/decoder.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/atomic_simple_cpu_exec.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/o3_cpu_exec.fo] Error 1
>>> scons: *** [build/POWER_SE/arch/power/timing_simple_cpu_exec.fo] Error 1
>>> scons: *** [build/POWER_SE/sim/faults.fo] Error 1
>>> scons: *** [build/POWER_SE/sim/stat_control.fo] Error 1
>>> scons: *** [build/POWER_SE/sim/pseudo_inst.fo] Error 1
>>> scons: *** [build/POWER_SE/sim/system.fo] Error 1
>>> scons: *** [build/POWER_SE/sim/tlb.fo] Error 1
>>> scons: *** [build/POWER_SE/sim/syscall_emul.fo] Error 1
>>> scons: *** [build/POWER_SE/sim/process.fo] Error 1
>>> scons: *** [build/POWER_SE/mem/physical.fo] Error 1
>>> scons: *** [build/POWER_SE/mem/page_table.fo] Error 1
>>> scons: *** [build/POWER_SE/mem/translating_port.fo] Error 1
>>> scons: *** [build/POWER_SE/mem/cache/base.fo] Error 1
>>> scons: *** [build/POWER_SE/mem/cache/prefetch/base.fo] Error 1
>>> scons: *** [build/POWER_SE/cpu/base.fo] Error 1
>>> scons: *** [build/POWER_SE/cpu/exetrace.fo] Error

Re: [m5-dev] Checkpointing

[Steve Reinhardt]

On Sun, Mar 27, 2011 at 1:27 PM, nathan binkert  wrote:
>> Is there any reason to have a serialize function in the timing and o3 cpus? 
>> Creating a checkpoint from them will be broken since if you're using cache 
>> the dirty data won't be saved? Shouldn't we change their implementation to 
>> fatal()?
>
> Is the implementation of the CPUs correct?  Arguably, it should be the
> caches that cause fatal() if they're what cause the problem, no?

I agree with Nate... in fact the Ruby caches do have a warm-up
facility that Brad is working on porting (or says he will), so we
don't want to assume that caches can't be checkpointed.  Also it's
possible to have a timing CPU with no caches (even though it doesn't
make a lot of sense).

What I would like to see is to have the O3 unserialize function fixed
so that we can avoid the silly switch_cpus thing when you want to
restore directly into O3... at least my understanding is that that's
why we don't do it that way.

Steve
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] changeset in m5: tests: update reference outputs for ruby cache ...

[Steve Reinhardt]

changeset 832ae3727c2b in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=832ae3727c2b
description:
tests: update reference outputs for ruby cache index change

MOESI_CMP_token is the only protocol that showed noticeable stats
differences.

diffstat:

 
tests/quick/00.hello/ref/alpha/linux/simple-timing-ruby-MOESI_CMP_token/config.ini
 | 5 +-
 
tests/quick/00.hello/ref/alpha/linux/simple-timing-ruby-MOESI_CMP_token/ruby.stats
 |   318 +-
 tests/quick/00.hello/ref/alpha/linux/simple-timing-ruby-MOESI_CMP_token/simout 
| 9 +-
 
tests/quick/00.hello/ref/alpha/linux/simple-timing-ruby-MOESI_CMP_token/stats.txt
  |16 +-
 
tests/quick/00.hello/ref/alpha/tru64/simple-timing-ruby-MOESI_CMP_token/config.ini
 | 5 +-
 
tests/quick/00.hello/ref/alpha/tru64/simple-timing-ruby-MOESI_CMP_token/ruby.stats
 |   334 +-
 tests/quick/00.hello/ref/alpha/tru64/simple-timing-ruby-MOESI_CMP_token/simout 
| 9 +-
 
tests/quick/00.hello/ref/alpha/tru64/simple-timing-ruby-MOESI_CMP_token/stats.txt
  |16 +-
 tests/quick/50.memtest/ref/alpha/linux/memtest-ruby-MOESI_CMP_token/config.ini 
|10 +-
 tests/quick/50.memtest/ref/alpha/linux/memtest-ruby-MOESI_CMP_token/ruby.stats 
|  1198 +-
 tests/quick/50.memtest/ref/alpha/linux/memtest-ruby-MOESI_CMP_token/simerr 
|   146 +-
 tests/quick/50.memtest/ref/alpha/linux/memtest-ruby-MOESI_CMP_token/simout 
| 9 +-
 tests/quick/50.memtest/ref/alpha/linux/memtest-ruby-MOESI_CMP_token/stats.txt  
|42 +-
 
tests/quick/60.rubytest/ref/alpha/linux/rubytest-ruby-MOESI_CMP_token/config.ini
   | 3 +-
 
tests/quick/60.rubytest/ref/alpha/linux/rubytest-ruby-MOESI_CMP_token/ruby.stats
   |   453 +-
 tests/quick/60.rubytest/ref/alpha/linux/rubytest-ruby-MOESI_CMP_token/simout   
| 9 +-
 
tests/quick/60.rubytest/ref/alpha/linux/rubytest-ruby-MOESI_CMP_token/stats.txt 
   | 8 +-
 17 files changed, 1301 insertions(+), 1289 deletions(-)

diffs (truncated from 4387 to 300 lines):

diff -r f789b9aac5f4 -r 832ae3727c2b 
tests/quick/00.hello/ref/alpha/linux/simple-timing-ruby-MOESI_CMP_token/config.ini
--- 
a/tests/quick/00.hello/ref/alpha/linux/simple-timing-ruby-MOESI_CMP_token/config.ini
Sat Mar 26 09:23:52 2011 -0400
+++ 
b/tests/quick/00.hello/ref/alpha/linux/simple-timing-ruby-MOESI_CMP_token/config.ini
Sat Mar 26 22:24:36 2011 -0700
@@ -63,7 +63,7 @@
 env=
 errout=cerr
 euid=100
-executable=/proj/aatl_perfmod_arch/m5_system_files/regression/test-progs/hello/bin/alpha/linux/hello
+executable=tests/test-progs/hello/bin/alpha/linux/hello
 gid=100
 input=cin
 max_stack_size=67108864
@@ -174,7 +174,7 @@
 latency=10
 replacement_policy=PSEUDO_LRU
 size=512
-start_index_bit=0
+start_index_bit=6
 
 [system.physmem]
 type=PhysicalMemory
@@ -208,6 +208,7 @@
 icache=system.l1_cntrl0.L1IcacheMemory
 max_outstanding_requests=16
 physmem=system.physmem
+using_network_tester=false
 using_ruby_tester=false
 version=0
 physMemPort=system.physmem.port[0]
diff -r f789b9aac5f4 -r 832ae3727c2b 
tests/quick/00.hello/ref/alpha/linux/simple-timing-ruby-MOESI_CMP_token/ruby.stats
--- 
a/tests/quick/00.hello/ref/alpha/linux/simple-timing-ruby-MOESI_CMP_token/ruby.stats
Sat Mar 26 09:23:52 2011 -0400
+++ 
b/tests/quick/00.hello/ref/alpha/linux/simple-timing-ruby-MOESI_CMP_token/ruby.stats
Sat Mar 26 22:24:36 2011 -0700
@@ -34,7 +34,7 @@
  End RubySystem Configuration Print 
 
 
-Real time: Feb/08/2011 17:51:05
+Real time: Mar/26/2011 22:00:44
 
 Profiler Stats
 --
@@ -43,20 +43,20 @@
 Elapsed_time_in_hours: 0
 Elapsed_time_in_days: 0
 
-Virtual_time_in_seconds: 0.4
-Virtual_time_in_minutes: 0.0067
-Virtual_time_in_hours:   0.00011
-Virtual_time_in_days:4.62963e-06
+Virtual_time_in_seconds: 0.23
+Virtual_time_in_minutes: 0.0038
+Virtual_time_in_hours:   6.38889e-05
+Virtual_time_in_days:2.66204e-06
 
-Ruby_current_time: 243131
+Ruby_current_time: 217591
 Ruby_start_time: 0
-Ruby_cycles: 243131
+Ruby_cycles: 217591
 
-mbytes_resident: 37.0508
-mbytes_total: 210.492
-resident_ratio: 0.176057
+mbytes_resident: 38.1094
+mbytes_total: 199.473
+resident_ratio: 0.19107
 
-ruby_cycles_executed: [ 243132 ]
+ruby_cycles_executed: [ 217592 ]
 
 Busy Controller Counts:
 L1Cache-0:0  
@@ -70,13 +70,13 @@
 
 All Non-Zero Cycle Demand Cache Accesses
 
-miss_latency: [binsize: 2 max: 277 count: 8464 average: 27.7253 | standard 
deviation: 60.1519 | 0 7084 0 0 0 0 0 0 0 0 79 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 202 178 134 156 352 4 6 4 3 8 40 31 65 31 60 0 0 0 0 1 
2 1 3 3 2 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 0 0 0 0 1 0 0 1 2 0 4 5 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 0 0 0 0 ]
-miss_latency_IFETCH: [binsize: 2 max: 205 c

Re: [m5-dev] changeset in m5: ruby: fixed cache index setting

[Steve Reinhardt]

Does it make sense that MOESI_CMP_token is the only protocol affected?

On Sat, Mar 26, 2011 at 5:39 PM, Beckmann, Brad  wrote:
> Thanks!
>
> Brad
>
>
>> -Original Message-
>> From: m5-dev-boun...@m5sim.org [mailto:m5-dev-boun...@m5sim.org] On
>> Behalf Of Steve Reinhardt
>> Sent: Saturday, March 26, 2011 1:54 PM
>> To: M5 Developer List
>> Subject: Re: [m5-dev] changeset in m5: ruby: fixed cache index setting
>>
>> I can do it... just wanted to make sure it was expected and not an
>> actual bug.
>>
>> On Sat, Mar 26, 2011 at 1:46 PM, Beckmann, Brad 
>> wrote:
>> > Hi Steve,
>> >
>> > Oops.  It was such a small change in configuration, I didn't think
>> about rerunning the regression tester, but now thinking about it, yes
>> it could impact the results.  The cache indexing functions were not
>> using the right bits before this change.
>> >
>> > I can go ahead and update the stats tonight.  However, let me know if
>> it is more convenient for you to update them yourself.
>> >
>> > Brad
>> >
>> >
>> >> -Original Message-
>> >> From: m5-dev-boun...@m5sim.org [mailto:m5-dev-boun...@m5sim.org] On
>> >> Behalf Of Steve Reinhardt
>> >> Sent: Saturday, March 26, 2011 6:20 AM
>> >> To: M5 Developer List
>> >> Subject: Re: [m5-dev] changeset in m5: ruby: fixed cache index
>> setting
>> >>
>> >> Hi Brad,
>> >>
>> >> Would you expect this to change the results for the ruby regressions
>> >> slightly?  The regressions passed last night because the tests
>> didn't
>> >> actually get rerun (since scons doesn't see the config file as a
>> >> dependency), but I'm seeing some failures in the tip on tests I'm
>> >> running and I suspect it's due to this change.
>> >>
>> >> Steve
>> >>
>> >> On Fri, Mar 25, 2011 at 10:15 AM, Brad Beckmann
>> 
>> >> wrote:
>> >> > changeset d8587c913ccf in /z/repo/m5
>> >> > details: http://repo.m5sim.org/m5?cmd=changeset;node=d8587c913ccf
>> >> > description:
>> >> >        ruby: fixed cache index setting
>> >> >
>> >> > diffstat:
>> >> >
>> >> >  configs/ruby/MESI_CMP_directory.py  |  17 +++--
>> >> >  configs/ruby/MI_example.py          |   4 +++-
>> >> >  configs/ruby/MOESI_CMP_directory.py |  17 +++--
>> >> >  configs/ruby/MOESI_CMP_token.py     |  15 +--
>> >> >  configs/ruby/MOESI_hammer.py        |  10 +++---
>> >> >  5 files changed, 41 insertions(+), 22 deletions(-)
>> >> >
>> >> > diffs (207 lines):
>> >> >
>> >> > diff -r bbab80b639cb -r d8587c913ccf
>> >> configs/ruby/MESI_CMP_directory.py
>> >> > --- a/configs/ruby/MESI_CMP_directory.py        Fri Mar 25
>> 00:46:14
>> >> 2011 -0400
>> >> > +++ b/configs/ruby/MESI_CMP_directory.py        Fri Mar 25
>> 10:13:50
>> >> 2011 -0700
>> >> > @@ -68,15 +68,19 @@
>> >> >     # Must create the individual controllers before the network to
>> >> ensure the
>> >> >     # controller constructors are called before the network
>> >> constructor
>> >> >     #
>> >> > +    l2_bits = int(math.log(options.num_l2caches, 2))
>> >> > +    block_size_bits = int(math.log(options.cacheline_size, 2))
>> >> >
>> >> >     for i in xrange(options.num_cpus):
>> >> >         #
>> >> >         # First create the Ruby objects associated with this cpu
>> >> >         #
>> >> >         l1i_cache = L1Cache(size = options.l1i_size,
>> >> > -                            assoc = options.l1i_assoc)
>> >> > +                            assoc = options.l1i_assoc,
>> >> > +                            start_index_bit = block_size_bits)
>> >> >         l1d_cache = L1Cache(size = options.l1d_size,
>> >> > -                            assoc = options.l1d_assoc)
>> >> > +                            assoc = options.l1d_assoc,
>> >> > +                            start_index_bit = block_size_bits)
>> >> >
>> >> >         cpu_seq = RubySequencer(version = i,
>> >> >            

Re: [m5-dev] Review Request: config: revamp x86 config to avoid appending to SimObjectVectors

[Steve Reinhardt]

> On 2011-03-26 12:31:49, Gabe Black wrote:
> > src/arch/x86/bios/E820.py, line 53
> > <http://reviews.m5sim.org/r/609/diff/1/?file=11254#file11254line53>
> >
> > I think at least most of these lists should be allowed to be empty 
> > regardless of if they're being appended to.

They're still allowed to be empty... the question is, does that make sense as a 
default value?  Would the system work with these parameters left as the empty 
list?  If so, I can put that back.


- Steve


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/609/#review1019
-------


On 2011-03-26 12:17:28, Steve Reinhardt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/609/
> ---
> 
> (Updated 2011-03-26 12:17:28)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> config: revamp x86 config to avoid appending to SimObjectVectors
> A significant contributor to the need for adoptOrphanParams()
> is the practice of appending to SimObjectVectors which have
> already been assigned as children.  This practice sidesteps the
> assignment operation for those appended SimObjects, which is
> where parent/child relationships are typically established.
> 
> This patch reworks the config scripts that use append() on
> SimObjectVectors, which all happen to be in the x86 system
> configuration.  At some point in the future, I hope to make
> SimObjectVectors immutable (by deriving from tuple rather than
> list), at which time this patch will be necessary for correct
> operation.  For now, it just avoids some of the warning
> messages that get printed in adoptOrphanParams().
> 
> 
> Diffs
> -
> 
>   configs/common/FSConfig.py d8587c913ccf 
>   src/arch/x86/bios/E820.py d8587c913ccf 
>   src/dev/x86/SouthBridge.py d8587c913ccf 
> 
> Diff: http://reviews.m5sim.org/r/609/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Steve
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] changeset in m5: ruby: fixed cache index setting

[Steve Reinhardt]

I can do it... just wanted to make sure it was expected and not an actual bug.

On Sat, Mar 26, 2011 at 1:46 PM, Beckmann, Brad  wrote:
> Hi Steve,
>
> Oops.  It was such a small change in configuration, I didn't think about 
> rerunning the regression tester, but now thinking about it, yes it could 
> impact the results.  The cache indexing functions were not using the right 
> bits before this change.
>
> I can go ahead and update the stats tonight.  However, let me know if it is 
> more convenient for you to update them yourself.
>
> Brad
>
>
>> -Original Message-
>> From: m5-dev-boun...@m5sim.org [mailto:m5-dev-boun...@m5sim.org] On
>> Behalf Of Steve Reinhardt
>> Sent: Saturday, March 26, 2011 6:20 AM
>> To: M5 Developer List
>> Subject: Re: [m5-dev] changeset in m5: ruby: fixed cache index setting
>>
>> Hi Brad,
>>
>> Would you expect this to change the results for the ruby regressions
>> slightly?  The regressions passed last night because the tests didn't
>> actually get rerun (since scons doesn't see the config file as a
>> dependency), but I'm seeing some failures in the tip on tests I'm
>> running and I suspect it's due to this change.
>>
>> Steve
>>
>> On Fri, Mar 25, 2011 at 10:15 AM, Brad Beckmann 
>> wrote:
>> > changeset d8587c913ccf in /z/repo/m5
>> > details: http://repo.m5sim.org/m5?cmd=changeset;node=d8587c913ccf
>> > description:
>> >        ruby: fixed cache index setting
>> >
>> > diffstat:
>> >
>> >  configs/ruby/MESI_CMP_directory.py  |  17 +++--
>> >  configs/ruby/MI_example.py          |   4 +++-
>> >  configs/ruby/MOESI_CMP_directory.py |  17 +++--
>> >  configs/ruby/MOESI_CMP_token.py     |  15 +--
>> >  configs/ruby/MOESI_hammer.py        |  10 +++---
>> >  5 files changed, 41 insertions(+), 22 deletions(-)
>> >
>> > diffs (207 lines):
>> >
>> > diff -r bbab80b639cb -r d8587c913ccf
>> configs/ruby/MESI_CMP_directory.py
>> > --- a/configs/ruby/MESI_CMP_directory.py        Fri Mar 25 00:46:14
>> 2011 -0400
>> > +++ b/configs/ruby/MESI_CMP_directory.py        Fri Mar 25 10:13:50
>> 2011 -0700
>> > @@ -68,15 +68,19 @@
>> >     # Must create the individual controllers before the network to
>> ensure the
>> >     # controller constructors are called before the network
>> constructor
>> >     #
>> > +    l2_bits = int(math.log(options.num_l2caches, 2))
>> > +    block_size_bits = int(math.log(options.cacheline_size, 2))
>> >
>> >     for i in xrange(options.num_cpus):
>> >         #
>> >         # First create the Ruby objects associated with this cpu
>> >         #
>> >         l1i_cache = L1Cache(size = options.l1i_size,
>> > -                            assoc = options.l1i_assoc)
>> > +                            assoc = options.l1i_assoc,
>> > +                            start_index_bit = block_size_bits)
>> >         l1d_cache = L1Cache(size = options.l1d_size,
>> > -                            assoc = options.l1d_assoc)
>> > +                            assoc = options.l1d_assoc,
>> > +                            start_index_bit = block_size_bits)
>> >
>> >         cpu_seq = RubySequencer(version = i,
>> >                                 icache = l1i_cache,
>> > @@ -91,9 +95,7 @@
>> >                                       sequencer = cpu_seq,
>> >                                       L1IcacheMemory = l1i_cache,
>> >                                       L1DcacheMemory = l1d_cache,
>> > -                                      l2_select_num_bits = \
>> > -
>>  math.log(options.num_l2caches,
>> > -                                                 2))
>> > +                                      l2_select_num_bits = l2_bits)
>> >
>> >         exec("system.l1_cntrl%d = l1_cntrl" % i)
>> >
>> > @@ -103,12 +105,15 @@
>> >         cpu_sequencers.append(cpu_seq)
>> >         l1_cntrl_nodes.append(l1_cntrl)
>> >
>> > +    l2_index_start = block_size_bits + l2_bits
>> > +
>> >     for i in xrange(options.num_l2caches):
>> >         #
>> >         # First create the Ruby objects associated with this cpu
>> >         #
>> >         l2_cache = L2Cache(size = options.l2_size,
>> > -                           assoc = options.l2_as

[m5-dev] Review Request: config: revamp x86 config to avoid appending to SimObjectVectors

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/609/
---

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

config: revamp x86 config to avoid appending to SimObjectVectors
A significant contributor to the need for adoptOrphanParams()
is the practice of appending to SimObjectVectors which have
already been assigned as children.  This practice sidesteps the
assignment operation for those appended SimObjects, which is
where parent/child relationships are typically established.

This patch reworks the config scripts that use append() on
SimObjectVectors, which all happen to be in the x86 system
configuration.  At some point in the future, I hope to make
SimObjectVectors immutable (by deriving from tuple rather than
list), at which time this patch will be necessary for correct
operation.  For now, it just avoids some of the warning
messages that get printed in adoptOrphanParams().


Diffs
-

  configs/common/FSConfig.py d8587c913ccf 
  src/arch/x86/bios/E820.py d8587c913ccf 
  src/dev/x86/SouthBridge.py d8587c913ccf 

Diff: http://reviews.m5sim.org/r/609/diff


Testing
---


Thanks,

Steve

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Review Request: sim: reinstate implicit parenting on parameter assignment

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/608/
---

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

sim: reinstate implicit parenting on parameter assignment
Last summer's big rewrite of the initialization code (in
particular cset 6efc3672733b) got rid of the implicit parenting
that used to occur when an unparented SimObject was assigned as
a parameter value to another SimObject.  The idea was that the
new adoptOrphanParams() step would catch these anyway so it was
unnecessary.

Unfortunately it turns out that adoptOrphanParams() has some
inherent instability in that the parent that does the adoption
depends on the config tree traversal order.  Even making this
order deterministic (e.g., by traversing children in
alphabetical order) can introduce unwanted and unexpected
hierarchy changes between similar configs (e.g., when adding a
switch_cpu in place of a cpu), causing problems when trying to
restore checkpoints across similar configs.  The hierarchy
created by implicit parenting is more stable and more
controllable, so this patch turns that behavior back on.

This patch also cleans up some long-standing holes regarding
parenting of SimObjects that are created in class definitions
(either in the body of the class, or as default parameters).

To avoid breaking some existing config files, this necessitated
changing the error on reparenting children to a warning.  This
change fixes another bug where attempting to print the prior
error message would fail on reparenting SimObjectVectors
because they lack a _parent attribute.  Some further issues
with SimObjectVectors were cleaned up by getting rid of the
get_parent() call (which could cause errors with some
SimObjectVectors where there was no single parent to return)
with has_parent() (since all the uses of get_parent() were just
boolean tests anyway).

Finally, since the adoptOrphanParam() step turned out to be so
problematic, we now issue a warning when it actually has to do
an adoption.  Future cleanup of config files will get rid of
current warnings.


Diffs
-

  src/python/m5/SimObject.py d8587c913ccf 
  src/python/m5/params.py d8587c913ccf 

Diff: http://reviews.m5sim.org/r/608/diff


Testing
---


Thanks,

Steve

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev