> On May 25, 2012, 10:10 a.m., Ali Saidi wrote:
> > Seems reasonable, although there doesn't seem to be much code shared
> > between the two buses. Mabye there just isn't much similar functionality?
> >
> > I'd like steve to sign off before committing.
> >
> >
>
> Nilay Vaish wrote:
> The coherent bus class inherits from the non-coherent bus class. So the
> functionality is being
> shared between the two.
>
> Andreas, from the posted patch it is not clear how much of the code is
> new, what code has been
> retained from the Bus class. Do you think there is a better way to
> display the patch? It seems
> reviewboard does not handle renaming of files that well.
Essentially non of the code is new. As mentioned in the comments, the two files
are copies of bus.{hh,cc}, and the only larger change are the simplifications
of the non-coherent bus which now does not have to deal with any snoopers and
forwarding. All I can suggest is download the patch and look at it with hg
diff. Other ideas?
In terms of Ali's question, most of the bus functionality is indeed shared
through the inheritance. I did consider having an abstract "BaseBus" and then
implement the transport functions in NoncoherentBus and CoherentBus, but at
this point it felt like a bit too much object orientation. I'd be happy to
change though.
- Andreas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/1223/#review2807
-----------------------------------------------------------
On May 25, 2012, 9:49 a.m., Andreas Hansson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1223/
> -----------------------------------------------------------
>
> (Updated May 25, 2012, 9:49 a.m.)
>
>
> Review request for Default.
>
>
> Description
> -------
>
> Changeset 9028:cd897893780f
> ---------------------------
> Bus: Split the bus into a non-coherent and coherent bus
>
> This patch introduces a class hierarchy of buses, a non-coherent one,
> and a coherent one, splitting the existing bus functionality. By doing
> so it also enables further specialisation of the two types of buses.
>
> A non-coherent bus connects a number of non-snooping masters and
> slaves, and routes the request and response packets based on the
> address. The request packets issued by the master connected to a
> non-coherent bus could still snoop in caches attached to a coherent
> bus, as is the case with the I/O bus and memory bus in most system
> configurations. No snoops will, however, reach any master on the
> non-coherent bus itself. The non-coherent bus can be used as a
> template for modelling PCI, PCIe, and non-coherent AMBA and OCP buses,
> and is typically used for the I/O buses.
>
> A coherent bus connects a number of (potentially) snooping masters and
> slaves, and routes the request and response packets based on the
> address, and also forwards all requests to the snoopers and deals with
> the snoop responses. The coherent bus can be used as a template for
> modelling QPI, HyperTransport, ACE and coherent OCP buses, and is
> typically used for the L1-to-L2 buses and as the main system
> interconnect.
>
> The configuration scripts are updated to use a NoncoherentBus for all
> peripheral and I/O buses.
>
> A bit of minor tidying up has also been done.
>
>
> Diffs
> -----
>
> configs/common/CacheConfig.py bb25e7646c41
> configs/common/FSConfig.py bb25e7646c41
> configs/example/memtest.py bb25e7646c41
> configs/example/se.py bb25e7646c41
> configs/splash2/cluster.py bb25e7646c41
> configs/splash2/run.py bb25e7646c41
> src/cpu/BaseCPU.py bb25e7646c41
> src/mem/Bus.py bb25e7646c41
> src/mem/SConscript bb25e7646c41
> src/mem/bus.hh bb25e7646c41
> src/mem/bus.cc bb25e7646c41
> src/mem/coherent_bus.hh PRE-CREATION
> src/mem/coherent_bus.cc PRE-CREATION
> src/mem/noncoherent_bus.hh PRE-CREATION
> src/mem/noncoherent_bus.cc PRE-CREATION
> tests/configs/inorder-timing.py bb25e7646c41
> tests/configs/memtest.py bb25e7646c41
> tests/configs/o3-timing-checker.py bb25e7646c41
> tests/configs/o3-timing-mp-ruby.py bb25e7646c41
> tests/configs/o3-timing-mp.py bb25e7646c41
> tests/configs/o3-timing-ruby.py bb25e7646c41
> tests/configs/o3-timing.py bb25e7646c41
> tests/configs/pc-o3-timing.py bb25e7646c41
> tests/configs/pc-simple-atomic.py bb25e7646c41
> tests/configs/pc-simple-timing.py bb25e7646c41
> tests/configs/realview-o3-checker.py bb25e7646c41
> tests/configs/realview-o3-dual.py bb25e7646c41
> tests/configs/realview-o3.py bb25e7646c41
> tests/configs/realview-simple-atomic-dual.py bb25e7646c41
> tests/configs/realview-simple-atomic.py bb25e7646c41
> tests/configs/realview-simple-timing-dual.py bb25e7646c41
> tests/configs/realview-simple-timing.py bb25e7646c41
> tests/configs/simple-atomic-dummychecker.py bb25e7646c41
> tests/configs/simple-atomic-mp-ruby.py bb25e7646c41
> tests/configs/simple-atomic-mp.py bb25e7646c41
> tests/configs/simple-atomic.py bb25e7646c41
> tests/configs/simple-timing-mp.py bb25e7646c41
> tests/configs/simple-timing.py bb25e7646c41
> tests/configs/tsunami-inorder.py bb25e7646c41
> tests/configs/tsunami-o3-dual.py bb25e7646c41
> tests/configs/tsunami-o3.py bb25e7646c41
> tests/configs/tsunami-simple-atomic-dual.py bb25e7646c41
> tests/configs/tsunami-simple-atomic.py bb25e7646c41
> tests/configs/tsunami-simple-timing-dual.py bb25e7646c41
> tests/configs/tsunami-simple-timing.py bb25e7646c41
>
> Diff: http://reviews.gem5.org/r/1223/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
