Steve Reinhardt wrote:
> On Sat, Aug 14, 2010 at 9:43 AM, Gabe Black <[email protected]> wrote:
>
>>>>>> @@ -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.
>>
>
> Actually I was originally referring to both parts; your explanation
> made sense for one part but not the other. I'd be happy to have you
> address the issue :-).
>
> Steve
> _______________________________________________
> m5-dev mailing list
> [email protected]
> http://m5sim.org/mailman/listinfo/m5-dev
>
Attached is a patch. I'd set up postreview, but I don't know all the
settings to use in my .hgrc (example, anyone? Maybe for the wiki?) and I
-am- supposed to be on vacation this weekend so I'm allowed to be lazy
:-). I don't imagine anyone's life will be lost if it waits until next
week some time to be reviewed and/or pushed.
Gabe
CPU: Set a default value when readBytes faults.
This was being done in read(), but if readBytes was called directly it
wouldn't happen. Also, instead of setting the memory blob being read to -1
which would (I believe) require using memset with -1 as a parameter, this now
uses bzero. It's hoped that it's more specialized behavior will make it
slightly faster.
diff -r 84fd1726290d -r 686dbf0a2563 src/cpu/base_dyn_inst.hh
--- a/src/cpu/base_dyn_inst.hh Sat Aug 14 01:00:45 2010 -0700
+++ b/src/cpu/base_dyn_inst.hh Sat Aug 14 08:17:58 2010 -0700
@@ -899,6 +899,12 @@
this->setExecuted();
}
+ if (fault != NoFault) {
+ // Return a fixed value to keep simulation deterministic even
+ // along misspeculated paths.
+ bzero(data, size);
+ }
+
if (traceData) {
traceData->setAddr(addr);
}
@@ -913,11 +919,6 @@
{
Fault fault = readBytes(addr, (uint8_t *)&data, sizeof(T), flags);
- if (fault != NoFault) {
- // Return a fixed value to keep simulation deterministic even
- // along misspeculated paths.
- data = (T)-1;
- }
data = TheISA::gtoh(data);
if (traceData) {
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev
