> On Dec. 17, 2013, 9:06 p.m., Joel Hestness wrote: > > ext/mcpat/cacti/Ucache.h, line 1 > > <http://reviews.gem5.org/r/2117/diff/1/?file=38654#file38654line1> > > > > After perusing the mcpat/cacti/* file changes, it appears that most of > > these changes are code formatting, so I'm not sure that they are necessary. > > The Cacti code included with McPAT is an already-lightly-modified version > > of the external tool (v6.5 I believe?), and there are arguments to be made > > for avoiding farther modification. For example, it may be desirable to > > incorporate a newer version of Cacti into McPAT at a later time, and more > > divergence from the Cacti point release will make it harder to identify the > > functional changes that may need to be resolved for McPAT. > > > > *NOTE: It's possible that I've missed necessary functional changes to > > Cacti in this review, so a more careful review may be necessary. > > > > Does anyone else have input on whether to make the Cacti changes?
You bring up a good point here. I don't think we need to make changes to this current patch, but going forward, we may not necessarily want to make these sorts of chages *if* we end up integrating a newer version of Cacti. Instead, I'd like to see us concentrate on making sure the interface between Cacti and McPAT is clean and generic. In the future, we may want to replace Cacti with other models for certain structures. - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2117/#review4830 ----------------------------------------------------------- On Jan. 6, 2014, 7:45 p.m., Yasuko Eckert wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2117/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2014, 7:45 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 9997:f7297b5e4e35 > --------------------------- > ext: McPAT interface changes and fixes > This patch includes software engineering changes and some generic bug fixes > Joel Hestness and Yasuko Eckert made to McPAT 0.8. There are still known > issues/concernts we did not have a chance to address in this patch. > > High-level changes in this patch include: > 1) Making XML parsing modular and hierarchical: > - Shift parsing responsibility into the components > - Read XML in a (mostly) context-free recursive manner so that McPAT input > files can contain arbitrary component hierarchies > 2) Making power, energy, and area calculations a hierarchical and recursive > process > - Components track their subcomponents and recursively call compute > functions in stages > - Make C++ object hierarchy reflect inheritance of classes of components > with similar structures > - Simplify computeArea() and computeEnergy() functions to eliminate > successive calls to calculate separate TDP vs. runtime energy > - Remove Processor component (now unnecessary) and introduce a more > abstract > System component > 3) Standardizing McPAT output across all components > - Use a single, common data structure for storing and printing McPAT output > - Recursively call print functions through component hierarchy > 4) For caches, allow splitting data array and tag array reads and writes for > better accuracy > 5) Improving the usability of CACTI by printing more helpful warning and > error > messages > 6) Minor: Impose more rigorous code style for clarity (more work still to be > done) > Overall, these changes greatly reduce the amount of replicated code, and they > improve McPAT runtime and decrease memory footprint. > > > Diffs > ----- > > ext/mcpat/XML_Parse.h PRE-CREATION > ext/mcpat/XML_Parse.cc PRE-CREATION > ext/mcpat/array.h PRE-CREATION > ext/mcpat/array.cc PRE-CREATION > ext/mcpat/basic_components.h PRE-CREATION > ext/mcpat/basic_components.cc PRE-CREATION > ext/mcpat/bus_interconnect.h PRE-CREATION > ext/mcpat/bus_interconnect.cc PRE-CREATION > ext/mcpat/cachearray.h PRE-CREATION > ext/mcpat/cachearray.cc PRE-CREATION > ext/mcpat/cachecontroller.h PRE-CREATION > ext/mcpat/cachecontroller.cc PRE-CREATION > ext/mcpat/cacheunit.h PRE-CREATION > ext/mcpat/cacheunit.cc PRE-CREATION > ext/mcpat/cacti/Ucache.h PRE-CREATION > ext/mcpat/cacti/Ucache.cc PRE-CREATION > ext/mcpat/cacti/arbiter.cc PRE-CREATION > ext/mcpat/cacti/bank.h PRE-CREATION > ext/mcpat/cacti/bank.cc PRE-CREATION > ext/mcpat/cacti/basic_circuit.h PRE-CREATION > ext/mcpat/cacti/basic_circuit.cc PRE-CREATION > ext/mcpat/cacti/cacti_interface.h PRE-CREATION > ext/mcpat/cacti/cacti_interface.cc PRE-CREATION > ext/mcpat/cacti/component.h PRE-CREATION > ext/mcpat/cacti/component.cc PRE-CREATION > ext/mcpat/cacti/const.h PRE-CREATION > ext/mcpat/cacti/crossbar.h PRE-CREATION > ext/mcpat/cacti/crossbar.cc PRE-CREATION > ext/mcpat/cacti/decoder.h PRE-CREATION > ext/mcpat/cacti/decoder.cc PRE-CREATION > ext/mcpat/cacti/htree2.h PRE-CREATION > ext/mcpat/cacti/htree2.cc PRE-CREATION > ext/mcpat/cacti/io.cc PRE-CREATION > ext/mcpat/cacti/mat.h PRE-CREATION > ext/mcpat/cacti/mat.cc PRE-CREATION > ext/mcpat/cacti/nuca.h PRE-CREATION > ext/mcpat/cacti/nuca.cc PRE-CREATION > ext/mcpat/cacti/parameter.h PRE-CREATION > ext/mcpat/cacti/parameter.cc PRE-CREATION > ext/mcpat/cacti/router.h PRE-CREATION > ext/mcpat/cacti/router.cc PRE-CREATION > ext/mcpat/cacti/subarray.h PRE-CREATION > ext/mcpat/cacti/subarray.cc PRE-CREATION > ext/mcpat/cacti/technology.cc PRE-CREATION > ext/mcpat/cacti/uca.h PRE-CREATION > ext/mcpat/cacti/uca.cc PRE-CREATION > ext/mcpat/cacti/wire.h PRE-CREATION > ext/mcpat/cacti/wire.cc PRE-CREATION > ext/mcpat/common.h PRE-CREATION > ext/mcpat/core.h PRE-CREATION > ext/mcpat/core.cc PRE-CREATION > ext/mcpat/globalvar.h PRE-CREATION > ext/mcpat/interconnect.h PRE-CREATION > ext/mcpat/interconnect.cc PRE-CREATION > ext/mcpat/iocontrollers.h PRE-CREATION > ext/mcpat/iocontrollers.cc PRE-CREATION > ext/mcpat/logic.h PRE-CREATION > ext/mcpat/logic.cc PRE-CREATION > ext/mcpat/main.cc PRE-CREATION > ext/mcpat/mcpat.mk PRE-CREATION > ext/mcpat/mcpatXeonCore.mk PRE-CREATION > ext/mcpat/memoryctrl.h PRE-CREATION > ext/mcpat/memoryctrl.cc PRE-CREATION > ext/mcpat/noc.h PRE-CREATION > ext/mcpat/noc.cc PRE-CREATION > ext/mcpat/processor.h PRE-CREATION > ext/mcpat/processor.cc PRE-CREATION > ext/mcpat/sharedcache.h PRE-CREATION > ext/mcpat/sharedcache.cc PRE-CREATION > ext/mcpat/system.h PRE-CREATION > ext/mcpat/system.cc PRE-CREATION > ext/mcpat/technology_xeon_core.cc PRE-CREATION > ext/mcpat/xmlParser.h PRE-CREATION > ext/mcpat/xmlParser.cc PRE-CREATION > > Diff: http://reviews.gem5.org/r/2117/diff/ > > > Testing > ------- > > > Thanks, > > Yasuko Eckert > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
