> On 2011-01-09 18:39:35, Ali Saidi wrote: > > src/sim/pseudo_inst.cc, line 329 > > <http://reviews.m5sim.org/r/418/diff/1/?file=9397#file9397line329> > > > > There needs to be a good comment about why you would use this here and > > the wiki page needs to be updated > > Brad Beckmann wrote: > I'm confused. These work completed functions are following the same > convention as other pseudo instructions that involve the thread context. > Both sets of functions do most of their logic in pseudo_inst. Why should the > guts of the work functions be moved to system? Or are you suggesting a > different change? > > Ali Saidi wrote: > I'm not suggesting that they're moved to system. I'm just saying that > they need a good comment that describes their purpose and use. > > Brad Beckmann wrote: > I added a few more comments, but I'm not sure if it is what you are > looking for. It seems a little overkill to put more any comments into > pseudo_inst.cc, since no other functions in this file are commented at all. > I do agree that we need to document on the wiki about how to use these > annotations. I think we should include that in our planned overhaul of the > wiki, since I don't see a good place to put it right now. Please let me know > if you have a good suggestion.
Thanks. Looks good to me. I realize that there aren't any other instructions in there that are documented, but I think that is an oversight. After digging through src/base it became clear to me that we write code knowing exactly what it does and what it should be used for and then forget about it. I think the comments you created would actually be better as doxygen comments in system.hh for incWorkItemsBegin() incWorkItemsEnd() and markWorkItem(). Both places might be good. Thanks again. - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/418/#review686 ----------------------------------------------------------- On 2011-01-10 16:21:58, Brad Beckmann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/418/ > ----------------------------------------------------------- > > (Updated 2011-01-10 16:21:58) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > m5: added work completed monitoring support > > > Diffs > ----- > > configs/common/FSConfig.py 9f9e10967912 > configs/common/Options.py 9f9e10967912 > configs/example/fs.py 9f9e10967912 > configs/example/ruby_fs.py 9f9e10967912 > src/arch/x86/isa/decoder/two_byte_opcodes.isa 9f9e10967912 > src/cpu/base.hh 9f9e10967912 > src/cpu/base.cc 9f9e10967912 > src/sim/SConscript 9f9e10967912 > src/sim/System.py 9f9e10967912 > src/sim/pseudo_inst.hh 9f9e10967912 > src/sim/pseudo_inst.cc 9f9e10967912 > src/sim/system.hh 9f9e10967912 > src/sim/system.cc 9f9e10967912 > util/m5/m5op_x86.S 9f9e10967912 > util/m5/m5ops.h 9f9e10967912 > > Diff: http://reviews.m5sim.org/r/418/diff > > > Testing > ------- > > > Thanks, > > Brad > >
_______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
