----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2117/#review4853 -----------------------------------------------------------
Ship it! This is looking good to me. One minor comment below, but I'm happy with this patch. Would be nice to have another party's input before committing. ext/mcpat/core.h <http://reviews.gem5.org/r/2117/#comment4542> After reviewing the use of ithCore, now, I think there is a better way to handle its use here (and possibly in other components); If a component should have an ID number, it should be included in the input XML and be the responsibility of the input generator to assign unique IDs for components as desired. This would continue the push toward completely context-free component instantiation. That said, I don't think such a change is necessary for this patch. Can you please add a comment here: "TODO: Migrate component ID handling into the XML data to remove this ithCore variable". - Joel Hestness On Dec. 24, 2013, 12:18 a.m., Yasuko Eckert wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2117/ > ----------------------------------------------------------- > > (Updated Dec. 24, 2013, 12:18 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 9997:2ccd199eb76c > --------------------------- > 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/cacti/io.cc PRE-CREATION > ext/mcpat/cacti/htree2.cc PRE-CREATION > ext/mcpat/cacti/htree2.h PRE-CREATION > ext/mcpat/cacti/decoder.cc PRE-CREATION > ext/mcpat/cacti/decoder.h PRE-CREATION > ext/mcpat/cacti/crossbar.cc PRE-CREATION > ext/mcpat/cacti/crossbar.h PRE-CREATION > ext/mcpat/cacti/const.h PRE-CREATION > ext/mcpat/cacti/component.cc PRE-CREATION > ext/mcpat/cacti/component.h PRE-CREATION > ext/mcpat/cacti/cacti_interface.cc PRE-CREATION > ext/mcpat/cacti/cacti_interface.h PRE-CREATION > ext/mcpat/cacti/basic_circuit.cc PRE-CREATION > ext/mcpat/cacti/basic_circuit.h PRE-CREATION > ext/mcpat/cacti/bank.cc PRE-CREATION > ext/mcpat/cacti/bank.h PRE-CREATION > ext/mcpat/cacti/arbiter.cc PRE-CREATION > ext/mcpat/cacti/Ucache.cc PRE-CREATION > ext/mcpat/cacti/Ucache.h PRE-CREATION > ext/mcpat/cacheunit.h PRE-CREATION > ext/mcpat/cacheunit.cc PRE-CREATION > ext/mcpat/cachecontroller.cc PRE-CREATION > ext/mcpat/cachecontroller.h PRE-CREATION > ext/mcpat/cachearray.cc PRE-CREATION > ext/mcpat/cachearray.h PRE-CREATION > ext/mcpat/bus_interconnect.cc PRE-CREATION > ext/mcpat/bus_interconnect.h PRE-CREATION > ext/mcpat/basic_components.cc PRE-CREATION > ext/mcpat/basic_components.h PRE-CREATION > ext/mcpat/array.cc PRE-CREATION > ext/mcpat/array.h PRE-CREATION > ext/mcpat/XML_Parse.cc PRE-CREATION > ext/mcpat/XML_Parse.h 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
