> On Nov. 12, 2015, 8:08 p.m., Joel Hestness wrote: > > src/mem/slicc/symbols/StateMachine.py, line 313 > > <http://reviews.gem5.org/r/3192/diff/2/?file=51487#file51487line313> > > > > Not sure if this should be fixed before commit, but I feel it should be > > fixed at some point: > > > > It seems like we should be using better inheritance here. For the > > interface with a cache state machine, is there a difference between a > > Sequencer and a Coalescer? If not, these getCPUSequencer and > > getGPUCoalescer functions should be merged to return a type from which both > > Sequencer and GPUCoalescer descend (e.g. in review request 3189, > > GPUCoalescer copies much of the Sequencer functionality, so it seems like > > GPUCoalescer should descend from Sequencer and just overload > > functionality). Elsewhere in this patch where you need to decide whether to > > accumulate sequencer or coalescer stats, we could use an accessor function > > in the common ancestor type that indicates whether the attached > > RubyPort/Sequencer is a "sequencer" or "coalescer". The interfaces for > > these types appear nearly identical, and this would keep the modifications > > to SLICC to a minimum (e.g. these changes would not be necessary).
There is a difference between the GPUCoalescer and Sequencer APIs. Though they have similar named functions, they require different arguments and behave very differently. Also note that it GPUCoalescer and/or Sequencer pointers are passed into the machines, not RubyPort pointers. I think it would be very hard to change that. The GPUCoalescer behaves very differently than the Sequencer. If you actually diff the cc files, you will notice that well over 90% of the lines are different and that no two functions are the same. There is some Alpha LLSC cruft in the GPUCoalescer that doesn't make much sense to be there. Perhaps that is why you think there is a lot of copying between the two files? Would you feel better about this patch if we removed the LLSC cruft from the GPUCoalescer in 3189? - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3192/#review7540 ----------------------------------------------------------- On Nov. 9, 2015, 9:16 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3192/ > ----------------------------------------------------------- > > (Updated Nov. 9, 2015, 9:16 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11199:5d5bd1b0d332 > --------------------------- > ruby: split CPU and GPU latency stats > > > Diffs > ----- > > src/mem/ruby/profiler/Profiler.hh 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > src/mem/ruby/profiler/Profiler.cc 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > src/mem/ruby/slicc_interface/AbstractController.hh > 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > src/mem/ruby/slicc_interface/AbstractController.cc > 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > src/mem/slicc/symbols/StateMachine.py > 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > > Diff: http://reviews.gem5.org/r/3192/diff/ > > > Testing > ------- > > > Thanks, > > Tony Gutierrez > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
