I should have noticed this the first time around... this fix doesn't work in the sense that whether or not a request is uncacheable is typically determined during translation (i.e., cacheability is often an attribute of the PTE), so even though moving this test up above the translation avoids the low-level error of referencing the possibly deleted Request object, you're now checking the cacheability bit before it has a chance to get set properly.
It's possibly irrelevant on two counts: 1. I have an alternate proposal for fixing this and the related traceData problem, which I'll send out in a separate email, and 2. I'd like to propose getting rid of this whole recordEvent() thing... I only see it used for uncached accesses in SimpleCPU (not in Inorder or O3) and for faults, and I'm not clear why we have it. Does anyone use these? Steve On Sun, Mar 21, 2010 at 10:47 PM, Brad Beckmann <[email protected]> wrote: > changeset 8b2b8e5e7d35 in /z/repo/m5 > details: http://repo.m5sim.org/m5?cmd=changeset;node=8b2b8e5e7d35 > description: > TimingSimpleCPU: Fixed uncacacheable request read bug > > Previously the recording of an uncached read occurred after the > request was > possibly deleted within the translateTiming function. > > diffstat: > > 1 file changed, 8 insertions(+), 9 deletions(-) > src/cpu/simple/timing.cc | 17 ++++++++--------- > > diffs (55 lines): > > diff -r 6c91d41dfc12 -r 8b2b8e5e7d35 src/cpu/simple/timing.cc > --- a/src/cpu/simple/timing.cc Sun Mar 21 21:22:20 2010 -0700 > +++ b/src/cpu/simple/timing.cc Sun Mar 21 21:22:20 2010 -0700 > @@ -432,6 +432,10 @@ > Addr split_addr = roundDown(addr + data_size - 1, block_size); > assert(split_addr <= addr || split_addr - addr < block_size); > > + // This will need a new way to tell if it's hooked up to a cache or not. > + if (req->isUncacheable()) > + recordEvent("Uncached Write"); > + > _status = DTBWaitResponse; > if (split_addr > addr) { > RequestPtr req1, req2; > @@ -461,10 +465,6 @@ > traceData->setAddr(addr); > } > > - // This will need a new way to tell if it has a dcache attached. > - if (req->isUncacheable()) > - recordEvent("Uncached Read"); > - > return NoFault; > } > > @@ -510,7 +510,6 @@ > return read(addr, *(uint32_t*)&data, flags); > } > > - > template<> > Fault > TimingSimpleCPU::read(Addr addr, int32_t &data, unsigned flags) > @@ -555,6 +554,10 @@ > Addr split_addr = roundDown(addr + data_size - 1, block_size); > assert(split_addr <= addr || split_addr - addr < block_size); > > + // This will need a new way to tell if it's hooked up to a cache or not. > + if (req->isUncacheable()) > + recordEvent("Uncached Write"); > + > T *dataP = new T; > *dataP = TheISA::htog(data); > _status = DTBWaitResponse; > @@ -586,10 +589,6 @@ > traceData->setData(data); > } > > - // This will need a new way to tell if it's hooked up to a cache or not. > - if (req->isUncacheable()) > - recordEvent("Uncached Write"); > - > // If the write needs to have a fault on the access, consider calling > // changeStatus() and changing it to "bad addr write" or something. > return NoFault; > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
