> On March 21, 2012, 10:12 p.m., Steve Reinhardt wrote: > > src/arch/arm/tlb.hh, line 218 > > <http://reviews.gem5.org/r/1091/diff/1/?file=24016#file24016line218> > > > > Why does this call return a pointer and not a reference? Seems > > inconsistent... > > Andreas Hansson wrote: > Inconsistent indeed. The TLB is the only place where the > getPort/getMasterPort is used to also determine if there is a port. The > BaseCPU calls this function and NULL is a valid return value. The alternative > is to introduce a "hasMasterPort" and the conventional &getMasterPort(). > > I opted for the easier way, but I'd be happy to change.
This is fine, but it's probably worth a comment in the code to explain the inconsistency. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1091/#review2353 ----------------------------------------------------------- On March 10, 2012, 11:56 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1091/ > ----------------------------------------------------------- > > (Updated March 10, 2012, 11:56 a.m.) > > > Review request for Default. > > > Description > ------- > > MEM: Introduce the master/slave port sub-classes in C++ > > This patch introduces the notion of a master and slave port in the C++ > code, thus bringing the previous classification from the Python > classes into the corresponding simulation objects and memory objects. > > The patch enables us to classify behaviours into the two bins and add > assumptions and enfore compliance, also simplifying the two > interfaces. As a starting point, isSnooping is confined to a master > port, and getAddrRanges to slave ports. More of these specilisations > are to come in later patches. > > The getPort function is not getMasterPort and getSlavePort, and > returns a port reference rather than a pointer as NULL would never be > a valid return value. The default implementation of these two > functions is placed in MemObject, and calls fatal. > > The one drawback with this specific patch is that it requires some > code duplication, e.g. QueuedPort becomes QueuedMasterPort and > QueuedSlavePort, and BusPort becomes BusMasterPort and BusSlavePort > (avoiding multiple inheritance). With the later introduction of the > port interfaces, moving the functionality outside the port itself, a > lot of the duplicated code will disappear again. > > > Diffs > ----- > > src/arch/arm/table_walker.hh 351585c17699 > src/arch/arm/table_walker.cc 351585c17699 > src/arch/arm/tlb.hh 351585c17699 > src/arch/arm/tlb.cc 351585c17699 > src/arch/x86/interrupts.hh 351585c17699 > src/arch/x86/interrupts.cc 351585c17699 > src/arch/x86/pagetable_walker.hh 351585c17699 > src/arch/x86/pagetable_walker.cc 351585c17699 > src/arch/x86/tlb.hh 351585c17699 > src/arch/x86/tlb.cc 351585c17699 > src/cpu/base.hh 351585c17699 > src/cpu/base.cc 351585c17699 > src/cpu/checker/cpu.hh 351585c17699 > src/cpu/inorder/resources/cache_unit.hh 351585c17699 > src/cpu/o3/cpu.hh 351585c17699 > src/cpu/o3/lsq_unit.hh 351585c17699 > src/cpu/o3/lsq_unit_impl.hh 351585c17699 > src/cpu/ozone/OzoneCPU.py 351585c17699 > src/cpu/ozone/cpu.hh 351585c17699 > src/cpu/ozone/cpu_impl.hh 351585c17699 > src/cpu/ozone/front_end.hh 351585c17699 > src/cpu/ozone/front_end_impl.hh 351585c17699 > src/cpu/ozone/lw_lsq.hh 351585c17699 > src/cpu/ozone/lw_lsq_impl.hh 351585c17699 > src/cpu/simple/atomic.hh 351585c17699 > src/cpu/simple/atomic.cc 351585c17699 > src/cpu/testers/directedtest/RubyDirectedTester.hh 351585c17699 > src/cpu/testers/directedtest/RubyDirectedTester.cc 351585c17699 > src/cpu/testers/memtest/memtest.hh 351585c17699 > src/cpu/testers/memtest/memtest.cc 351585c17699 > src/cpu/testers/networktest/networktest.hh 351585c17699 > src/cpu/testers/networktest/networktest.cc 351585c17699 > src/cpu/testers/rubytest/RubyTester.hh 351585c17699 > src/cpu/testers/rubytest/RubyTester.cc 351585c17699 > src/dev/copy_engine.hh 351585c17699 > src/dev/copy_engine.cc 351585c17699 > src/dev/io_device.hh 351585c17699 > src/dev/io_device.cc 351585c17699 > src/dev/pcidev.hh 351585c17699 > src/dev/x86/i82094aa.hh 351585c17699 > src/dev/x86/i82094aa.cc 351585c17699 > src/dev/x86/intdev.hh 351585c17699 > src/dev/x86/intdev.cc 351585c17699 > src/kern/tru64/tru64_events.cc 351585c17699 > src/mem/bridge.hh 351585c17699 > src/mem/bridge.cc 351585c17699 > src/mem/bus.hh 351585c17699 > src/mem/bus.cc 351585c17699 > src/mem/cache/base.hh 351585c17699 > src/mem/cache/base.cc 351585c17699 > src/mem/cache/builder.cc 351585c17699 > src/mem/cache/cache.hh 351585c17699 > src/mem/cache/cache_impl.hh 351585c17699 > src/mem/fs_translating_port_proxy.hh 351585c17699 > src/mem/fs_translating_port_proxy.cc 351585c17699 > src/mem/mem_object.hh 351585c17699 > src/mem/mem_object.cc 351585c17699 > src/mem/mport.hh 351585c17699 > src/mem/mport.cc 351585c17699 > src/mem/physical.hh 351585c17699 > src/mem/physical.cc 351585c17699 > src/mem/port.hh 351585c17699 > src/mem/port.cc 351585c17699 > src/mem/port_proxy.hh 351585c17699 > src/mem/qport.hh PRE-CREATION > src/mem/ruby/system/RubyPort.hh 351585c17699 > src/mem/ruby/system/RubyPort.cc 351585c17699 > src/mem/se_translating_port_proxy.hh 351585c17699 > src/mem/se_translating_port_proxy.cc 351585c17699 > src/mem/tport.hh 351585c17699 > src/mem/tport.cc 351585c17699 > src/python/swig/pyobject.cc 351585c17699 > src/sim/system.hh 351585c17699 > src/sim/system.cc 351585c17699 > src/sim/tlb.hh 351585c17699 > > Diff: http://reviews.gem5.org/r/1091/diff/ > > > Testing > ------- > > util/regress all passing (disregarding t1000 and eio) > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
