cpu. arch: add initiateMemRead() to ExecContext interface
Review Request #3269 - Created Dec. 31, 2015 and submitted
| Information | |
|---|---|
| Steve Reinhardt | |
| gem5 | |
| default | |
| Reviewers | |
| Default | |
Changeset 11287:e947130e058d --------------------------- cpu. arch: add initiateMemRead() to ExecContext interface For historical reasons, the ExecContext interface had a single function, readMem(), that did two different things depending on whether the ExecContext supported atomic memory mode (i.e., AtomicSimpleCPU) or timing memory mode (all the other models). In the former case, it actually performed a memory read; in the latter case, it merely initiated a read access, and the read completion did not happen until later when a response packet arrived from the memory system. This led to some confusing things, including timing accesses being required to provide a pointer for the return data even though that pointer was only used in atomic mode. This patch splits this interface, adding a new initiateMemRead() function to the ExecContext interface to replace the timing-mode use of readMem(). For consistency and clarity, the readMemTiming() helper function in the ISA definitions is renamed to initiateMemRead() as well. For x86, where the access size is passed in explicitly, we can also get rid of the data parameter at this level. For other ISAs, where the access size is determined from the type of the data parameter, we have to keep the parameter for that purpose.
Posted (Dec. 31, 2015, 4:43 a.m.)
This makes good sense -- the new naming makes more clear the behavior of memory reads in timing mode. A couple minor nits below.
-
src/cpu/exec_context.hh (Diff revision 1) -
I presume the write path doesn't get the same treatment because it does need the same prototype? Why not repeat the initiate/write refactoring for symmetry and clarity?
-
src/cpu/simple/atomic.cc (Diff revision 1) -
similar idea as comment on Timing
-
src/cpu/simple/timing.cc (Diff revision 1) -
I would prefer that it proclaim something like "readMem() (an atomic memory interface) should never be called on TimingSimpleCPU!"
Review request changed
Updated (Dec. 31, 2015, 8:55 a.m.)
Diff: |
Revision 2 (+77 -38) |
|---|
Review request changed
Updated (Dec. 31, 2015, 8:57 a.m.)
Posted (Jan. 11, 2016, 1:24 p.m.)
Any more comments on this? I would like to commit in the next few days, esp. since all the precursor changes (3266-3268) did get ship-its and are ready to go, and I'd like to commit this whole batch at once. Thanks!