Nilay, can you explain the impact of that bug in terms of simulation performance? Are benchmarks running slower because of this change? Will regressions need to be updated?
On Fri, May 6, 2011 at 8:13 PM, Beckmann, Brad <brad.beckm...@amd.com>wrote: > Hi Nilay, > > Yeah, pulling the State into the Machine makes sense to me. If I recall, > my previous patch made it necessary that each machine included a > state_declaration (previously the state enum). More tightly integrating the > state to the machine seems to be a natural progression on that path. > > I understand moving the permission settings back to setState is the easiest > way to make this work. However, it would be great if we could combine the > setting of state and the setting of permission into one function call from > the sm file. Thus we don't have to worry about the situation where one sets > the state, but forgets to set the permission. That could lead to some > random functional access failing and a very painful debug. > > Brad > > > > -----Original Message----- > > From: m5-dev-boun...@m5sim.org [mailto:m5-dev-boun...@m5sim.org] > > On Behalf Of Nilay Vaish > > Sent: Friday, May 06, 2011 3:52 PM > > To: Nilay Vaish; Default > > Subject: [m5-dev] Review Request: Ruby: Correctly set access permissions > > for directory entries > > > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > http://reviews.m5sim.org/r/684/ > > ----------------------------------------------------------- > > > > Review request for Default. > > > > > > Summary > > ------- > > > > Ruby: Correctly set access permissions for directory entries The access > > permissions for the directory entries are not being set correctly. > > This is because pointers are not used for handling directory entries. > > function. The setState() function will once again set the permissions as > well. > > But it would make use of the State_to_permission() function, instead of > > doing the analysis it used to do earlier. The changePermission() function > > provided by the AbstractEntry and AbstractCacheEntry classes has been > > exposed to SLICC code once again. The set_permission() functionality has > > been removed. > > > > I have done this only for the MESI protocol so far. Once we build a > consensus > > one the changes, I will make changes to other protocols as well. > > > > As far as testing is concerned, the protocol compiles and clears 10000 > loads. > > I did not test any more than that. > > > > A point that I wanted to raise for discussion: I think we should pull > State > > enum and the accompanying functions into the Machine it self. Brad, what > > do you think? > > > > > > Diffs > > ----- > > > > src/mem/protocol/MESI_CMP_directory-L1cache.sm 3c628a51f6e1 > > src/mem/protocol/MESI_CMP_directory-L2cache.sm 3c628a51f6e1 > > src/mem/protocol/MESI_CMP_directory-dir.sm 3c628a51f6e1 > > src/mem/protocol/RubySlicc_Types.sm 3c628a51f6e1 > > src/mem/slicc/ast/MethodCallExprAST.py 3c628a51f6e1 > > src/mem/slicc/symbols/StateMachine.py 3c628a51f6e1 > > > > Diff: http://reviews.m5sim.org/r/684/diff > > > > > > Testing > > ------- > > > > > > Thanks, > > > > Nilay > > > > _______________________________________________ > > m5-dev mailing list > > m5-dev@m5sim.org > > http://m5sim.org/mailman/listinfo/m5-dev > > > _______________________________________________ > m5-dev mailing list > m5-dev@m5sim.org > http://m5sim.org/mailman/listinfo/m5-dev > -- - Korey _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
