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

[m5-dev] Split Translation

2009-04-28 Thread nathan binkert
Hi Gabe, [ I wrote this e-mail almost a month and a half ago and I just noticed that it never got sent. I think it is still relevant] I've started looking at your patch and in particular some of the translation code in the TimingSimpleCPU. I have a couple of comments. 1) You call

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

Re: [m5-dev] Split Translation

2009-04-28 Thread Gabriel Michael Black
Quoting nathan binkert n...@binkert.org: Hi Gabe, [ I wrote this e-mail almost a month and a half ago and I just noticed that it never got sent. I think it is still relevant] I've started looking at your patch and in particular some of the translation code in the TimingSimpleCPU. I have

Re: [m5-dev] Split Translation

2009-04-28 Thread nathan binkert
[ I wrote this e-mail almost a month and a half ago and I just noticed that it never got sent.  I think it is still relevant] I've started looking at your patch and in particular some of the translation code in the TimingSimpleCPU.  I have a couple of comments. 1) You call

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 misses

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 n...@binkert.org: 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

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. I'm

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

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 was

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 ksew...@umich.edu: 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

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 now. If

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 n...@binkert.org: 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

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 where

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 have a

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] Split Translation

2009-04-28 Thread Steve Reinhardt
On Tue, Apr 28, 2009 at 1:19 PM, nathan binkert n...@binkert.org wrote: 2) You split the translation if the address crosses blocks. It seems that it'd be better to check to see if the access crosses pages, not blocks to save the extra work. I was told I should use the peer block size

Re: [m5-dev] Split Translation

2009-04-28 Thread nathan binkert
I don't see much benefit in doing the split twice (once for page crossings and a second time for cache line crossings), though if there was a way to isolate the bulk of the code in a single function that gets called twice it wouldn't be too bad.  There is some merit in only calling the TLB

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

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 gbl...@eecs.umich.edu 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?