----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/358/#review727 -----------------------------------------------------------
I can't fully appreciate these latest changes without also seeing the updated .sm files, but overall this looks very inline with what we have been discussing over the past few days. I just have one basic question on why we need to overload doTransition, getState, and setState. Why can't we just have one version? Also a possibly related idea is can you insert asserts (!= NULL) for the tbe and cache_entry pointers before they are dereferenced in the generated code? src/mem/slicc/ast/FuncCallExprAST.py <http://reviews.m5sim.org/r/358/#comment991> Why do we need four versions of doTransition? Can't we just pass in NULL/OOD for the tbe and cache_entry? src/mem/slicc/symbols/StateMachine.py <http://reviews.m5sim.org/r/358/#comment990> After you make your change, why would getState ever be called without the address, tbe, and cache_entry? Wouldn't you just pass in NULL/OOD if the tbe or cache_entry didn't exist? src/mem/slicc/symbols/StateMachine.py <http://reviews.m5sim.org/r/358/#comment989> After you make this change, why would setState ever be called without passing the address, tbe, and cache_entry? - Brad On 2011-01-12 22:45:22, Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/358/ > ----------------------------------------------------------- > > (Updated 2011-01-12 22:45:22) > > > Review request for Default. > > > Summary > ------- > > The purpose of this patch is to change the way CacheMemory interfaces with > coherence protocols. Currently, whenever a cache controller (defined in the > protocol under consideration) needs to carry out any operation on a cache > block, it looks up the tag hash map and figures out whether or not the block > exists in the cache. In case it does exist, the operation is carried out > (which requires another lookup). Over a single clock cycle, multiple such > lookups take place as observed through profiling of different protocols. It > was seen that the tag lookup takes anything from 10% to 20% of the simulation > time. In order to reduce this time, this patch is being posted. The > CacheMemory class now will have a function that will return pointer to a > cache block entry, instead of a reference (though the function that returns > the reference has been retained currently). Functions have been introduced > for setting/unsetting a pointer and check its pointer. Similar changes have > been carried out for Transaction Buffer entries as well. > > Note that changes are required to both SLICC and the protocol files. This > patch carries out changes to SLICC and committing this patch alone, I > believe, will ___break___ all the protocols. I am working on patching the > protocols as well. This patch is being put to get feed back from other > developers. > > > Diffs > ----- > > src/mem/protocol/RubySlicc_Types.sm c6bc8fe81e79 > src/mem/ruby/slicc_interface/AbstractCacheEntry.hh c6bc8fe81e79 > src/mem/ruby/slicc_interface/AbstractCacheEntry.cc c6bc8fe81e79 > src/mem/ruby/system/CacheMemory.hh c6bc8fe81e79 > src/mem/ruby/system/CacheMemory.cc c6bc8fe81e79 > src/mem/ruby/system/TBETable.hh c6bc8fe81e79 > src/mem/slicc/ast/ActionDeclAST.py c6bc8fe81e79 > src/mem/slicc/ast/FormalParamAST.py c6bc8fe81e79 > src/mem/slicc/ast/FuncCallExprAST.py c6bc8fe81e79 > src/mem/slicc/ast/IfStatementAST.py c6bc8fe81e79 > src/mem/slicc/ast/InPortDeclAST.py c6bc8fe81e79 > src/mem/slicc/ast/IsValidPtrExprAST.py PRE-CREATION > src/mem/slicc/ast/LocalVariableAST.py PRE-CREATION > src/mem/slicc/ast/MethodCallExprAST.py c6bc8fe81e79 > src/mem/slicc/ast/StaticCastAST.py c6bc8fe81e79 > src/mem/slicc/ast/TypeDeclAST.py c6bc8fe81e79 > src/mem/slicc/ast/__init__.py c6bc8fe81e79 > src/mem/slicc/parser.py c6bc8fe81e79 > src/mem/slicc/symbols/Func.py c6bc8fe81e79 > src/mem/slicc/symbols/StateMachine.py c6bc8fe81e79 > > Diff: http://reviews.m5sim.org/r/358/diff > > > Testing > ------- > > I have tested these changes using the MOESI_CMP_directory and MI protocols. > > > Thanks, > > Nilay > >
_______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
