> On 2011-01-13 08:33:49, Brad Beckmann wrote: > > 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?
Because Entry and/or TBE types might not be defined at all for particular controllers. Same holds true for the getState() and setState(). Also, getState() and setState() do not require the address, if the tbe and cache_entry have been properly set. > On 2011-01-13 08:33:49, Brad Beckmann wrote: > > src/mem/slicc/ast/FuncCallExprAST.py, line 127 > > <http://reviews.m5sim.org/r/358/diff/9/?file=9549#file9549line127> > > > > Why do we need four versions of doTransition? Can't we just pass in > > NULL/OOD for the tbe and cache_entry? Because Entry type might not be defined at all for that particular controller. Same holds true for the getState() and setState(). Also, getState() and setState() do not require the address, if the tbe and cache_entry have been properly set. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/358/#review727 ----------------------------------------------------------- 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 m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
