Re: [m5-dev] changeset in m5: O3CPU: Fix a bug where stores in the cpu where ...

2010-07-22 Thread Steve Reinhardt
(I wrote most of this email before I saw your second message, so I'm not sure whether it still applies or not, but I figured I'd send it anyway.) Hi Gabe, I certainly agree with your goals, but I will quibble with some of the details... On Thu, Jul 22, 2010 at 2:17 PM, Gabriel Michael Black wro

Re: [m5-dev] changeset in m5: O3CPU: Fix a bug where stores in the cpu where ...

2010-07-22 Thread Gabriel Michael Black
Actually, maybe this falls into a subcategory of 2. which is why it seems more acceptable. 2.a If the ISA says it's safe to use this optimization which doesn't affect behavior and which could be turned off and only result in lower simulator performance, skip this code/check/whatever. This

Re: [m5-dev] changeset in m5: O3CPU: Fix a bug where stores in the cpu where ...

2010-07-22 Thread Gabriel Michael Black
To me there's actually a spectrum of ISA dependence, incompletely described below: 1. If ISA == suchandsuch do this, otherwise do that. 2. If ISA has characteristic such and such, do this, otherwise do that. 3. Here, ISA, you take care of this. 4. ISA data parameterizing non-ISA stuff. Number

Re: [m5-dev] changeset in m5: O3CPU: Fix a bug where stores in the cpu where ...

2010-07-22 Thread Steve Reinhardt
In generalI think this is the kind of ISA hook we should be using... in the sense that checking TheISA::HasUnalignedMemAcc is much better than "(TheISA == x86 || TheISA == Power)". I think it's useful not only to avoid the overhead of a dynamic check for an ISA that doesn't need it, but also to cl

Re: [m5-dev] Review Request: BranchPred: Take the branch predictor out of O3CPU and make it a stand-alone

2010-07-22 Thread Timothy M Jones
On Thu, 22 Jul 2010 16:35:10 -0400, Steve Reinhardt wrote: On Thu, Jul 22, 2010 at 1:12 PM, Timothy M Jones wrote: So, after all this, which version do you want me to implement? TID or ASID? I'll have a go at either. I think you should go with the TID since that's the normal approa

Re: [m5-dev] Review Request: BranchPred: Take the branch predictor out of O3CPU and make it a stand-alone

2010-07-22 Thread Steve Reinhardt
On Thu, Jul 22, 2010 at 1:12 PM, Timothy M Jones wrote: > > So, after all this, which version do you want me to implement?  TID or ASID? >  I'll have a go at either. I think you should go with the TID since that's the normal approach. If Korey needs to use the ASID instead for his particular appl

Re: [m5-dev] Review Request: BranchPred: Take the branch predictor out of O3CPU and make it a stand-alone

2010-07-22 Thread Timothy M Jones
Hi Folks, On Sun, 18 Jul 2010 10:46:38 -0400, Korey Sewell wrote: That seems odd... I assume you're suggesting that Tim switch the InOrderCPU to pass the PC instead, right? Yep, I'm suggesting that this should be updated. Tim, you'll want to edit the code fetch_seq_unit.cc, execution_unit.c

Re: [m5-dev] changeset in m5: O3CPU: Fix a bug where stores in the cpu where ...

2010-07-22 Thread Gabriel Michael Black
Yes it does, and that sounds reasonable to me. I'd still like to see us use ISA hooks as minimally as possible, but this seems ok. Gabe Quoting Timothy M Jones : Oh, ok, I see where you're going with that. However, the main idea of having TheISA::HasUnalignedMemAcc was that it is a constan

Re: [m5-dev] changeset in m5: O3CPU: Fix a bug where stores in the cpu where ...

2010-07-22 Thread Timothy M Jones
Oh, ok, I see where you're going with that. However, the main idea of having TheISA::HasUnalignedMemAcc was that it is a constant specific to each ISA. Therefore, the compiler should really recognise this and optimise it away wherever it's used. In this case, for ISAs that don't have una

Re: [m5-dev] changeset in m5: Power: The condition register should be set or ...

2010-07-22 Thread Timothy M Jones
Ah, sorry, I'll be sure to take this into account in the future. Tim On Thu, 22 Jul 2010 14:29:44 -0400, Gabe Black wrote: In mercurial, the first line of a commit description is treated specially and should be a one line summary of the change. You're first line here and in a few other cha

Re: [m5-dev] changeset in m5: O3CPU: Fix a bug where stores in the cpu where ...

2010-07-22 Thread Gabe Black
I think you missed my point. If the check of TheISA::HasUnalignedMemAcc is redundant, we shouldn't be checking it at all. It's a free, though very small, performance bump, but more significantly it removes a direct dependence on the ISA. Gabe Timothy M. Jones wrote: > changeset 3bd51d6ac9ef in /z

Re: [m5-dev] changeset in m5: Power: The condition register should be set or ...

2010-07-22 Thread Gabe Black
In mercurial, the first line of a commit description is treated specially and should be a one line summary of the change. You're first line here and in a few other changes I noticed wraps to the second line which is a no-no. In the future, please keep it to one line even if you have to leave out de

[m5-dev] changeset in m5: Power: The condition register should be set or ...

2010-07-22 Thread Timothy M. Jones
changeset ffac9df60637 in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=ffac9df60637 description: Power: The condition register should be set or cleared upon a system call return to indicate success or failure. diffstat: src/arch/power/miscregs.hh | 7 ++-

[m5-dev] changeset in m5: LSQ Unit: After deleting part of a split reques...

2010-07-22 Thread Timothy M. Jones
changeset bd104adbf04d in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=bd104adbf04d description: LSQ Unit: After deleting part of a split request, set it to NULL so that it isn't accidentally deleted again later (causing a segmentation fault). diffstat: src/cp

[m5-dev] changeset in m5: Port: Only indicate that a SimpleTimingPort is ...

2010-07-22 Thread Timothy M. Jones
changeset fb7fc9aca918 in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=fb7fc9aca918 description: Port: Only indicate that a SimpleTimingPort is drained if its send event is not scheduled, as well as the transmit list being empty. diffstat: src/mem/cache/cache_

[m5-dev] changeset in m5: O3CPU: Fix a bug where stores in the cpu where ...

2010-07-22 Thread Timothy M. Jones
changeset 3bd51d6ac9ef in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=3bd51d6ac9ef description: O3CPU: Fix a bug where stores in the cpu where never marked as split. diffstat: src/cpu/o3/lsq_unit.hh | 6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diffs

[m5-dev] changeset in m5: Syscall: Don't close the simulator's standard f...

2010-07-22 Thread Timothy M. Jones
changeset 02b471d9d400 in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=02b471d9d400 description: Syscall: Don't close the simulator's standard file descriptors. diffstat: src/sim/syscall_emul.cc | 5 - 1 files changed, 4 insertions(+), 1 deletions(-) diffs (15 li

[m5-dev] changeset in m5: O3CPU: O3's tick event gets squashed when it is...

2010-07-22 Thread Timothy M. Jones
changeset b1ac6773e83d in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=b1ac6773e83d description: O3CPU: O3's tick event gets squashed when it is switched out. When repeatedly switching between O3 and another CPU, O3's tick event might still be scheduled

[m5-dev] changeset in m5: Power: Provide a utility function to copy regis...

2010-07-22 Thread Timothy M. Jones
changeset e76cc0ca16cc in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=e76cc0ca16cc description: Power: Provide a utility function to copy registers from one thread context to another in the Power ISA. diffstat: src/arch/power/SConscript | 1 + src/arch/powe

Re: [m5-dev] times syscall fix / ThreadContext suspension-reactivation

2010-07-22 Thread Steve Reinhardt
On Thu, Jul 22, 2010 at 10:03 AM, Korey Sewell wrote: > The problem is that there are some conventions that we would need to point > out and follow (i.e. Do threads start out Halted, Idle, or ???) Yea, I know that's the key to the problems we had before, is that different people had different men

Re: [m5-dev] times syscall fix / ThreadContext suspension-reactivation

2010-07-22 Thread Korey Sewell
On Thu, Jul 22, 2010 at 12:53 PM, nathan binkert wrote: > > This looks like a legit bug. In getting SMT to work as threads are > > spawned/respawned/suspended/etc. there may definitely be some bugs there > in > > code assuming the CPU only has 1 thread context. > > > > do you want to give fixing

Re: [m5-dev] times syscall fix / ThreadContext suspension-reactivation

2010-07-22 Thread nathan binkert
> This looks like a legit bug. In getting SMT to work as threads are > spawned/respawned/suspended/etc. there may definitely be some bugs there in > code assuming the CPU only has 1 thread context. > > do you want to give fixing that a go and then send out a patch to the m5-dev > list (or better ye

Re: [m5-dev] changeset in m5: stats: unify the two stats distribution type be...

2010-07-22 Thread Ali Saidi
This fixed my problem... Thanks Nate! Ali On Wed, 21 Jul 2010 21:54:56 -0400, Nathan Binkert begin_of_the_skype_highlighting end_of_the_skype_highlighting wrote: > changeset 7772a8bf76ee in /z/repo/m5 > details: http://repo.m5sim.org/m5?cmd=changeset;node=7772a8bf76ee > description: >

Re: [m5-dev] times syscall fix / ThreadContext suspension-reactivation

2010-07-22 Thread Korey Sewell
> Secondly, I have implemented the nanosleep() system call. I believe that > the appropriate way to do this would be suspending (tc->suspend()) the > ThreadContext and re-activating (tc->activate(delay)) after the sleep > duration (please correct me if I am mistaken). This however led to a > peculi

Re: [m5-dev] O3CPU + translateTiming

2010-07-22 Thread Korey Sewell
Eventually, the InOrderCPU's "DynInst" object will be merged with the base_dyn_inst.hh. If memory serves correct, in the original implementation I did not want to deal with the templated-definition and derivation of the base_dyn_inst.hh (at least upfront) and so DynInst specific to InOrderCPU was

[m5-dev] diff reviewing

2010-07-22 Thread nathan binkert
I just wanted to point out to everyone on the list that we welcome everyone's comments on reviewing patches (not just the core developers). So, if you see a patch that you are interested in for whatever reason, please review it. The more eyeballs on the code, the better. If there are features th

Re: [m5-dev] Review Request: O3CPU: Fix a bug where stores in the cpu where never marked as split.

2010-07-22 Thread Timothy Jones
> On 2010-07-22 00:26:07, Gabe Black wrote: > > src/cpu/o3/lsq_unit.hh, line 825 > > > > > > Is it possible for sreqLow to be non-null and > > TheISA::HasUnalignedMemAcc -not- to be true? In that instance, wouldn't > > this sti

Re: [m5-dev] Simulating caches having variable read and write latencies.

2010-07-22 Thread Steve Reinhardt
I don't know of any fundamental reason this shouldn't work, but I also doubt that anyone has tried it before, so it's not terribly surprising that you're running into bugs. In general you have the right idea on how to go about it. Steve On Sun, Jul 18, 2010 at 12:48 PM, Vidyabhushan Mohan wrote

Re: [m5-dev] times syscall fix / ThreadContext suspension-reactivation

2010-07-22 Thread Steve Reinhardt
For your second part (the thread suspend/activate) I agree that at first glance it looks like a bug. Korey has been the most recent person to work with O3 SMT thread handling, and I see his name attached to some of the lines in question, so I'll let him respond... Steve On Wed, Jul 21, 2010 at 2

Re: [m5-dev] O3CPU + translateTiming

2010-07-22 Thread Steve Reinhardt
I think you stated it pretty accurately, Gabe. It looks like the base_dyn_inst.hh file is only used in o3, ozone, and checker, and of those only o3 is really being used right now. Steve On Thu, Jul 22, 2010 at 12:56 AM, Gabe Black wrote: > The dynamic instruction object is really just the dynam

Re: [m5-dev] Review Request: util/m5/m5.c: in readfile(), added memset to touch all pages - ensure they are in the page table

2010-07-22 Thread Gabe Black
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/64/#review106 --- util/m5/Makefile.x86 Why

Re: [m5-dev] Booting Linux, X86_FS Timing CPU

2010-07-22 Thread Gabe Black
Ok, I've had a chance to look at this. The reason completeDataAccess is called twice is that the first instance is for the previous instruction. It's data access comes back, allowing it to finish, and because it finished it fires off the next instruction. I'm assuming this is a subsequent microop o

Re: [m5-dev] times syscall fix / ThreadContext suspension-reactivation

2010-07-22 Thread Gabe Black
Ioannis Ilkos wrote: > Hello, > > I have been playing with time in m5 syscall emulation mode and had a couple > of issues: > > First of all I believe there is a bug in the times() syscall implementation > (sim/syscall_emul.hh). The current clocks value passed to the userland is: > int64_t c

Re: [m5-dev] O3CPU + translateTiming

2010-07-22 Thread Gabe Black
The dynamic instruction object is really just the dynamic information associated with an instruction, as apposed to the static instruction object that gets reused. Strictly speaking there's no guarantee that an arbitrary dynamic instruction will always use timing mode, but all the CPU models we hav

Re: [m5-dev] Review Request: O3CPU: Fix a bug where stores in the cpu where never marked as split.

2010-07-22 Thread Gabe Black
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/54/#review105 --- src/cpu/o3/lsq_unit.hh Is

Re: [m5-dev] Review Request: Syscall: Don't close the simulator's standard file descriptors.

2010-07-22 Thread Gabe Black
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/52/#review104 --- src/sim/syscall_emul.cc W

[m5-dev] Notification from M5 Bugs

2010-07-22 Thread Flyspray
THIS IS AN AUTOMATED MESSAGE, DO NOT REPLY. A new Flyspray task has been opened. Details are below. User who did this: - Gabe Black (gblack) Attached to Project - M5 Bugs Summary - Create a "generic" ISA directory. Task Type - Minor Enhancement Category - ISA Support Status - New Assigned To -

Re: [m5-dev] Review Request: Power: Provide a utility function to copy registers from one thread context

2010-07-22 Thread Gabe Black
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/35/#review103 --- src/arch/power/utility.cc