Steve Reinhardt wrote: > 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. >
Oh, *that* part was what you were referring to. Yes, I wasn't quite sure what to do with that so I just got rid of it. I can easily believe that wasn't the right thing to do, so if you want me to put it back (or want to put it back yourself) I won't complain. > >>> 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. > Yeah, I was planning to use that information in the future but not to resend the review. Sorry about that. Gabe _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
