-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2875/#review6535
-----------------------------------------------------------



src/dev/dma_device.cc (line 97)
<http://reviews.gem5.org/r/2875/#comment5603>

    Check drainState() == Draining?



src/dev/i8254xGBe.cc (line 2130)
<http://reviews.gem5.org/r/2875/#comment5604>

    We can drop the true &&.



src/dev/i8254xGBe.cc (line 2229)
<http://reviews.gem5.org/r/2875/#comment5605>

    Same as above.



src/dev/i8254xGBe.cc (line 2442)
<http://reviews.gem5.org/r/2875/#comment5606>

    Same as above.



src/mem/dramsim2.cc (line 120)
<http://reviews.gem5.org/r/2875/#comment5607>

    Check drainState == draining?



src/mem/dramsim2.cc (line 338)
<http://reviews.gem5.org/r/2875/#comment5608>

    Same as above.



src/mem/ruby/system/RubyPort.cc (line 390)
<http://reviews.gem5.org/r/2875/#comment5611>

    drainState() == Draining



src/mem/simple_mem.cc (line 203)
<http://reviews.gem5.org/r/2875/#comment5610>

    drainState() == Draining



src/mem/simple_mem.cc (line 204)
<http://reviews.gem5.org/r/2875/#comment5609>

    Correct the spelling here? Draining not drainng



src/sim/drain.hh (line 232)
<http://reviews.gem5.org/r/2875/#comment5612>

    I think there is slight incorrect grammar here.  May be replace 
"consequently be stop being drained" with "consequently may need to be drained 
again."



src/sim/drain.cc (line 181)
<http://reviews.gem5.org/r/2875/#comment5613>

    This seems strange.  Should this not be _drainState = DrainState::Draining.


- Nilay Vaish


On June 8, 2015, 12:13 p.m., Andreas Sandberg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/2875/
> -----------------------------------------------------------
> 
> (Updated June 8, 2015, 12:13 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 10873:c844ebc70284
> ---------------------------
> sim: Refactor and simplify the drain API
> 
> The drain() call currently passes around a DrainManager pointer, which
> is now completely pointless since there is only ever one global
> DrainManager in the system. It also contains vestiges from the time
> when SimObjects had to keep track of their child objects that needed
> draining.
> 
> 
> This changeset moves all of the DrainState handling to the Drainable
> base class and changes the drain() and drainResume() calls to reflect
> this. Particularly, the drain() call has been updated to take no
> parameters (the DrainManager argument isn't needed) and return a
> DrainState instead of an unsigned integer (there is no point returning
> anything other than 0 or 1 any more). Drainable objects should return
> either DrainState::Draining (equivalent to returning 1 in the old
> system) if they need more time to drain or DrainState::Drained
> (equivalent to returning 0 in the old system) if they are already in a
> consistent state. Returning DrainState::Running is considered an
> error.
> 
> Drain done signalling is now done through the signalDrainDone() method
> in the Drainable class instead of using the DrainManager directly. The
> new call checks if the state of the object is DrainState::Draining
> before notifying the drain manager. This means that it is safe to call
> signalDrainDone() without first checking if the simulator has
> requested draining. The intention here is to reduce the code needed to
> implement draining in simple objects.
> 
> 
> Diffs
> -----
> 
>   src/mem/ruby/structures/RubyMemoryControl.cc 282c2a89ace8 
>   src/mem/ruby/system/DMASequencer.hh 282c2a89ace8 
>   src/mem/ruby/system/DMASequencer.cc 282c2a89ace8 
>   src/mem/ruby/system/RubyPort.hh 282c2a89ace8 
>   src/mem/ruby/system/RubyPort.cc 282c2a89ace8 
>   src/mem/dramsim2.cc 282c2a89ace8 
>   src/mem/packet_queue.hh 282c2a89ace8 
>   src/mem/packet_queue.cc 282c2a89ace8 
>   src/mem/ruby/structures/RubyMemoryControl.hh 282c2a89ace8 
>   src/dev/ns_gige.cc 282c2a89ace8 
>   src/dev/sinic.hh 282c2a89ace8 
>   src/dev/sinic.cc 282c2a89ace8 
>   src/mem/cache/mshr_queue.hh 282c2a89ace8 
>   src/mem/cache/mshr_queue.cc 282c2a89ace8 
>   src/mem/dram_ctrl.hh 282c2a89ace8 
>   src/mem/dram_ctrl.cc 282c2a89ace8 
>   src/mem/dramsim2.hh 282c2a89ace8 
>   src/dev/i8254xGBe.cc 282c2a89ace8 
>   src/dev/ide_disk.cc 282c2a89ace8 
>   src/dev/ns_gige.hh 282c2a89ace8 
>   src/dev/dma_device.hh 282c2a89ace8 
>   src/dev/dma_device.cc 282c2a89ace8 
>   src/dev/i8254xGBe.hh 282c2a89ace8 
>   src/dev/arm/ufs_device.hh 282c2a89ace8 
>   src/dev/arm/ufs_device.cc 282c2a89ace8 
>   src/dev/copy_engine.hh 282c2a89ace8 
>   src/dev/copy_engine.cc 282c2a89ace8 
>   src/arch/arm/table_walker.hh 282c2a89ace8 
>   src/arch/arm/table_walker.cc 282c2a89ace8 
>   src/arch/arm/tlb.hh 282c2a89ace8 
>   src/cpu/kvm/base.hh 282c2a89ace8 
>   src/cpu/kvm/base.cc 282c2a89ace8 
>   src/cpu/minor/cpu.hh 282c2a89ace8 
>   src/cpu/minor/cpu.cc 282c2a89ace8 
>   src/cpu/minor/pipeline.hh 282c2a89ace8 
>   src/cpu/minor/pipeline.cc 282c2a89ace8 
>   src/cpu/o3/cpu.hh 282c2a89ace8 
>   src/cpu/o3/cpu.cc 282c2a89ace8 
>   src/cpu/simple/atomic.hh 282c2a89ace8 
>   src/cpu/simple/atomic.cc 282c2a89ace8 
>   src/cpu/simple/timing.hh 282c2a89ace8 
>   src/cpu/simple/timing.cc 282c2a89ace8 
>   src/cpu/testers/traffic_gen/traffic_gen.hh 282c2a89ace8 
>   src/cpu/testers/traffic_gen/traffic_gen.cc 282c2a89ace8 
>   src/dev/arm/flash_device.hh 282c2a89ace8 
>   src/dev/arm/flash_device.cc 282c2a89ace8 
>   src/sim/system.hh 282c2a89ace8 
>   src/sim/system.cc 282c2a89ace8 
>   src/sim/drain.cc 282c2a89ace8 
>   src/sim/process.hh 282c2a89ace8 
>   src/sim/process.cc 282c2a89ace8 
>   src/sim/sim_object.hh 282c2a89ace8 
>   src/sim/sim_object.cc 282c2a89ace8 
>   src/mem/ruby/system/Sequencer.cc 282c2a89ace8 
>   src/mem/simple_mem.hh 282c2a89ace8 
>   src/mem/simple_mem.cc 282c2a89ace8 
>   src/mem/xbar.hh 282c2a89ace8 
>   src/mem/xbar.cc 282c2a89ace8 
>   src/sim/drain.hh 282c2a89ace8 
> 
> Diff: http://reviews.gem5.org/r/2875/diff/
> 
> 
> Testing
> -------
> 
> Regressions pass. Minor stats difference (due to changes in drain call order) 
> in one of the pc switcheroo tests.
> 
> 
> Thanks,
> 
> Andreas Sandberg
> 
>

_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to