I'd advise against putting the comments in two places since they probably wouldn't stay in sync/up to date. You could refer from one comment to the other, though, and have it in two places through indirection.
Gabe On 01/10/11 16:45, Ali Saidi wrote: > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/418/ > > > On January 9th, 2011, 6:39 p.m., *Ali Saidi* wrote: > > src/sim/pseudo_inst.cc > <http://reviews.m5sim.org/r/418/diff/1/?file=9397#file9397line329> > (Diff revision 1) > > debugbreak(ThreadContext *tc) > > > > 329 > > void > > There needs to be a good comment about why you would use this here > and the wiki page needs to be updated > > On January 10th, 2011, 11:48 a.m., *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? > > On January 10th, 2011, 3:50 p.m., *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. > > On January 10th, 2011, 4:28 p.m., *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 > > > On January 10th, 2011, 4:21 p.m., Brad Beckmann wrote: > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, > and Nathan Binkert. > By Brad Beckmann. > > /Updated 2011-01-10 16:21:58/ > > > Description > > 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) > > View Diff <http://reviews.m5sim.org/r/418/diff/> > > > _______________________________________________ > 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
