On Fri, Aug 13, 2010 at 10:54 PM, Gabe Black <[email protected]> wrote: > Steve Reinhardt wrote: >> On Fri, Aug 13, 2010 at 6:32 AM, Gabe Black <[email protected]> wrote: >> >>> changeset 67c670459d01 in /z/repo/m5 >>> details: http://repo.m5sim.org/m5?cmd=changeset;node=67c670459d01 >>> description: >>> CPU: Add readBytes and writeBytes functions to the exec contexts. >>> >>> >> >> >>> @@ -889,11 +894,6 @@ >>> effAddrValid = true; >>> fault = cpu->read(req, sreqLow, sreqHigh, data, lqIdx); >>> } else { >>> - >>> - // Return a fixed value to keep simulation deterministic even >>> - // along misspeculated paths. >>> - data = (T)-1; >>> - >>> // Commit will have to clean up whatever happened. Set this >>> // instruction as executed. >>> this->setExecuted(); >>> @@ -901,7 +901,6 @@ >>> >>> if (traceData) { >>> traceData->setAddr(addr); >>> - traceData->setData(data); >>> } >>> >>> return fault; >>> >> >> So I see that you moved these lines up to read(), but if we ever call >> readBytes() directly (not via read())---which I thought was why you >> needed to add this---then shouldn't the equivalent functionality stay >> down at this level? >> > > No. The data needs to be endian converted, and that doesn't make sense > at readBytes.
I can see where that applies for traceData->setData, but -1 is a pretty endian-agnostic value. I'm just concerned that other call sites for readBytes (which I assume are forthcoming) won't all remember that these two things need to be done. > >> Also, a little longer commit message warning us that you didn't just >> add these functions but also rewrote read() and write() in terms of >> them would have been helpful. >> > > I -did- send this out for review a while ago :-). Oh well. I guess there was a miscommunication... I know you sent out the patches via email, but I thought the upshot of that thread was that we had given you enough pointers on how to submit to reviewboard that you were going to try that again, so I was waiting for that to happen before I looked at them. Steve _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
