> On Dec. 31, 2015, 4:43 a.m., Curtis Dunham wrote: > > src/cpu/exec_context.hh, lines 203-206 > > <http://reviews.gem5.org/r/3269/diff/1/?file=52933#file52933line203> > > > > 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?
Yes, that's right. This all stems from implementing cmpxchg16b, which needs to perform 128-bit memory operations, and in the process of figuring out the x86 memory access path and enabling 128-bit operations I got to the point where it seemed silly to have to allocate a useless data buffer just to initiate a timing read. writeMem() doesn't force such contortions, so it didn't bother me, though I can see where it would be reasonable to split it similarly for symmetry. I'd rather not delay this patch to do so though. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3269/#review7798 ----------------------------------------------------------- On Dec. 31, 2015, 12:10 a.m., Steve Reinhardt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3269/ > ----------------------------------------------------------- > > (Updated Dec. 31, 2015, 12:10 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > 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. > > > Diffs > ----- > > src/arch/alpha/isa/mem.isa 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/arch/arm/isa/templates/mem.isa 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/arch/arm/isa/templates/mem64.isa > 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/arch/arm/isa/templates/neon64.isa > 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/arch/generic/memhelpers.hh 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/arch/mips/isa/formats/mem.isa 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/arch/power/isa/formats/mem.isa 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/arch/sparc/isa/formats/mem/util.isa > 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/arch/x86/isa/formats/monitor_mwait.isa > 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/arch/x86/isa/microops/ldstop.isa > 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/arch/x86/memhelpers.hh 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/cpu/base_dyn_inst.hh 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/cpu/exec_context.hh 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/cpu/minor/exec_context.hh 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/cpu/simple/atomic.hh 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/cpu/simple/atomic.cc 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/cpu/simple/base.hh 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/cpu/simple/exec_context.hh 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/cpu/simple/timing.hh 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > src/cpu/simple/timing.cc 4cc8b312f026321a81cb0b043e8e3a5c20b5f5db > > Diff: http://reviews.gem5.org/r/3269/diff/ > > > Testing > ------- > > > Thanks, > > Steve Reinhardt > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
