Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2010-03-22 Thread Gabe Black
Ok. This sounds plausible to me, and I'm glad it's not going to be so much work. I'm sure you already thought of this, but along the lines of what we were talking about in that other thread, something that important and subtle will deserve a big obvious comment or two. Gabe Steve Reinhardt wrote:

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2010-03-22 Thread Steve Reinhardt
Hi Gabe, On Sun, Mar 21, 2010 at 5:22 PM, Gabe Black wrote: > It's been a while since I've looked at this, but I want to make sure I > remember to respond so I don't want to wait until I have a chance to > re-research all this. Isn't the problem that initiateAcc ends up calling > completeAcc mid-

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2010-03-22 Thread Gabe Black
It's been a while since I've looked at this, but I want to make sure I remember to respond so I don't want to wait until I have a chance to re-research all this. Isn't the problem that initiateAcc ends up calling completeAcc mid-function, that cleans up after the access, then initateAcc gets contro

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2010-03-21 Thread Steve Reinhardt
Thanks to Brad's prompting, I took another look at this tonight. I think the fundamental problem is that the transition to split-transaction translation didn't move all the operations that depend on the translation completing into the translation callback or something that was guaranteed to run af

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Steve Reinhardt
On Tue, Apr 28, 2009 at 4:43 PM, Gabe Black wrote: > I'm not sure if it will be hugely better. I'd have to just do it and see > how it comes out, I suppose. Wouldn't scheduling an event be similar to > what I have, just using the event queue instead of the new packet > pointer? Yea, at a certai

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Gabe Black
Steve Reinhardt wrote: > Whew, I don't check my email for one afternoon and you guys make me > pay for it... > > I'm still curious about the possibility of using an event to call the > completeAcc() callback in the case where the translation fails on a > store conditional. This solution seems part

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Steve Reinhardt
Whew, I don't check my email for one afternoon and you guys make me pay for it... I'm still curious about the possibility of using an event to call the completeAcc() callback in the case where the translation fails on a store conditional. This solution seems particularly attractive to me if it's

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread nathan binkert
> So a tentative plan would be: > 1. Commit my patch for now so the bug is fixed. > 2. Add the #if TRACING_ON. > 3.5 Hash out what to do with traceData > 3.5 Reorganize the timing simple CPU. > > Does anyone object? I guess I'd rather not do #1 if it isn't the solution because things like that hav

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Gabriel Michael Black
Quoting nathan binkert : >> One completely different solution would be to do what you're saying >> and just not have the timing simple CPU work like it does. I think >> it's organized a lot like the atomic simple CPU because it used to be >> a part of it, but really a more natural organization wou

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread nathan binkert
> One completely different solution would be to do what you're saying > and just not have the timing simple CPU work like it does. I think > it's organized a lot like the atomic simple CPU because it used to be > a part of it, but really a more natural organization would be as a > state machine whe

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Gabriel Michael Black
Quoting nathan binkert : >>> I'm specifically talking about the parameter passed into the exec >>> function which is either SimpleCPU or BaseDynInst. Those both have a >>> traceData parameter and it can just be used directly without any fancy >>> indirection. I am only talking about this code sp

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread nathan binkert
>> I'm specifically talking about the parameter passed into the exec >> function which is either SimpleCPU or BaseDynInst.  Those both have a >> traceData parameter and it can just be used directly without any fancy >> indirection.  I am only talking about this code specifically. > > I'm confused n

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Gabriel Michael Black
Quoting Korey Sewell : > So my original point (that obviously wasnt conveyed very well) was > more trying to re-work how this STL_C case works rather than kind of > patch it up so it works in the current framework. > > Seems to be me that completeAcc should never get called before > initiateAcc in

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread nathan binkert
> Seems to be me that completeAcc should never get called before > initiateAcc in any circumstance. I know that shortcut is because you > STL_C fails early, but even so, why cant it just finish like any other > instruction. What benefit are we gettiing from early completing it > (especially in a Si

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Korey Sewell
> As far as moving around the code, that would be easier said than done. > That's all generated and I don't know how easy it would be to get the > isa_parser to generate something structured differently that would > still make sense in all the other instructions of all the other ISAs. > Steve? I w

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Korey Sewell
So my original point (that obviously wasnt conveyed very well) was more trying to re-work how this STL_C case works rather than kind of patch it up so it works in the current framework. Seems to be me that completeAcc should never get called before initiateAcc in any circumstance. I know that shor

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Gabriel Michael Black
Quoting nathan binkert : >>> What redirection? Or do you mean because traceData is protected in >>> the simple CPU? You could, but then again, you could just make it >>> public. Otherwise, I'm not sure what you're talking about. The >>> execute function is generated for each cpu model separate

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread nathan binkert
>> What redirection?  Or do you mean because traceData is protected in >> the simple CPU?  You could, but then again, you could just make it >> public.  Otherwise, I'm not sure what you're talking about.  The >> execute function is generated for each cpu model separately and it >> knows the type. >

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Gabriel Michael Black
Quoting nathan binkert : >>> I think the difference is that passing it in explicitly means that >>> you could >>> keep it in a register for the duration of the function, and if you can put >>> it in a callee-save then maybe you could save a few dereferences. In >>> contrast the indirection woul

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread nathan binkert
> This bug happens when both the translation component and the memory > access component finish inline before returning. When that happens, > completeAcc is effectively called from initiateAcc and you get into > trouble. > > The translation component will complete inline in all cases except TLB > m

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread nathan binkert
>> I think the difference is that passing it in explicitly means that you could >> keep it in a register for the duration of the function, and if you can put >> it in a callee-save then maybe you could save a few dereferences.  In >> contrast the indirection would have to be repeated after every fu

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Gabriel Michael Black
Quoting Steve Reinhardt : > On Tue, Apr 28, 2009 at 10:21 AM, nathan binkert wrote: > >> Realistically, I don't see it being more complicated since you'd just >> be replacing traceData with foo->traceData. Efficiency is >> questionable as well since that traceData had to come from somewhere. > >

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Gabriel Michael Black
This bug happens when both the translation component and the memory access component finish inline before returning. When that happens, completeAcc is effectively called from initiateAcc and you get into trouble. The translation component will complete inline in all cases except TLB misses

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Steve Reinhardt
On Tue, Apr 28, 2009 at 10:21 AM, nathan binkert wrote: > Realistically, I don't see it being more complicated since you'd just > be replacing traceData with foo->traceData. Efficiency is > questionable as well since that traceData had to come from somewhere. I think the difference is that pas

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread nathan binkert
> I don't know for sure, but I would guess that would be a lot less > efficient and more complicated. traceData gets used for every output > operand so you'd be calling through the exec context a lot, even when > it's just going to return NULL. This is how the isa_parser has > generated this code s

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-28 Thread Steve Reinhardt
OK, thinking about this a bit more, it seems like the root of the problem is that the timing translation is calling the callback right away instead of deferring it to a later cycle. I assume this only happens on a TLB miss for a store conditional? Is there any other path by which the completeAcc

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-27 Thread Gabriel Michael Black
Quoting nathan binkert : >> initiateAcc looks something like the following: >> >> Fault initiateAcc(..., traceData,...) >> { >>... >>write(); <<< traceData becomes invalid in here >>if (traceData) traceData->setData(Mem); >>... >> } >> >> so I'm not sure how that would work. > > Wh

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-27 Thread nathan binkert
> initiateAcc looks something like the following: > > Fault initiateAcc(..., traceData,...) > { >    ... >    write(); <<< traceData becomes invalid in here >    if (traceData) traceData->setData(Mem); >    ... > } > > so I'm not sure how that would work. Why is traceData passed in as a parameter

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-27 Thread Gabriel Michael Black
initiateAcc looks something like the following: Fault initiateAcc(..., traceData,...) { ... write(); <<< traceData becomes invalid in here if (traceData) traceData->setData(Mem); ... } so I'm not sure how that would work. Gabe Quoting nathan binkert : >> 7. initiateAcc uses

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-27 Thread nathan binkert
> 7. initiateAcc uses its local (and now invalid) copy of the traceData > pointer to store the result of the instruction executing. What if you just read this pointer again and not store a local copy? Nate ___ m5-dev mailing list m5-dev@m5sim.org http

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-27 Thread Gabriel Michael Black
Here is a hypothetical series of events that should hopefully illustrate what's going on. 1. The store conditional comes up for execution and its initiateAcc is called. 2. initiateAcc calls xc->write() to actually do the required store. 3. write calls into the TLB's translateTiming function to d

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-27 Thread Korey Sewell
Hi Gabe, I'm trying to figure out what's going and help find a potentially better solution (if there is one), since I'm operating under the assumption that adding a small overhead for the common case memory access to support the special case (stl_c) should be avoided if possible. (SIDEBAR: In looki

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-26 Thread Gabe Black
I should add, I realize it's not very clear what's going on, but it would take a lot more effort to explain everything in enough detail to get the problem across than for you (or anyone else) to look at the code. If you do look into the code, it's not a functional problem as far as how the timing C

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-26 Thread Gabe Black
You should read my description, read the code, and then once you understand the problem comment on my solution. Gabe Korey Sewell wrote: > Yea, I was on that thread. Your explanation wasnt particularly in > great detail there (files?, line #s?) so its hard to follow without > getting your hand di

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-26 Thread Korey Sewell
Yea, I was on that thread. Your explanation wasnt particularly in great detail there (files?, line #s?) so its hard to follow without getting your hand dirty on where this code is you are referring to. I'll comment and you can clarify my assumptions... "Oooh. I see what's broken. This is

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-26 Thread Gabe Black
There's an explanation already in the thread with the subject "Memory corruption inm5devrepositorywhenusing --trace-flags="ExecEnable"". Stalling or sleeping doesn't make sense in this situation since it's not that sort of problem. Gabe Korey Sewell wrote: > Can you explai

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-26 Thread Korey Sewell
Can you explain how the completeAcc ever gets called before the initiatieAcc finishes in a SimpleCPU? I'm thinking, why isnt the SimpleCPU just stalling if there is a delay waiting for a memory access.If that's the right behavior, then isnt the solution to just stall the CPU (or even sleep it) and

Re: [m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-26 Thread Gabe Black
This patch should fix the issue Geoff was having where tracing would cause a segfault. If you could please give it a try, Geoff, let me know if it actually solves the problem. This will add a small overhead for every memory instruction even though the old behavior is wrong only in certain circumsta

[m5-dev] [PATCH] CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc

2009-04-26 Thread gblack
# HG changeset patch # User Gabe Black # Date 1240729422 25200 # Node ID 284ff3c27bbddacf5763e7b5a54e99e4dcd1912a # Parent 8652636856b3d24fb0088fb1af5f5dca5008d9c8 CPU: Make the CPU wait until initiateAcc finishes before calling completeAcc. diff --git a/src/cpu/simple/timing.cc b/src/cpu/simple