> On 2010-10-21 13:35:21, Steve Reinhardt wrote: > > src/mem/protocol/MESI_CMP_directory-L1cache.sm, line 332 > > <http://reviews.m5sim.org/r/277/diff/1/?file=4455#file4455line332> > > > > This DPRINTF is broken (and several others like it)... it needs a trace > > flag as the first arg, and "%str" probably isn't the format string you want > > (basically it's just like printf, so you probably mean "%s"). > > Nilay Vaish wrote: > The DPRINTF statements that appear in .sm files are re-written by the > SLICC compiler. SLICC compiler provides the trace flag. Looking at the format > specifier, the compiler will add a .c_str() after print_str(). But in a > comment above, you mentioned that c_str() is not required. May be we can drop > %str altogether. I need to look in to this. > > Steve Reinhardt wrote: > OK, thanks for the clarification. I missed the SLICC compiler code > before but I found it now. I have two comments on that: > - Unless there's a real need to have a different syntax, it would be nice > to just write the SLICC code as a regular DPRINTF and pass DPRINTF calls > through unmodified. I'd like to better understand why you think it's needed > (if you still think it's needed after making the c_str() change). > - If we do end up needing a DPRINTF replacement with different syntax in > SLICC, let's call it something other than DPRINTF to make it clear that it is > different. > > > Nilay Vaish wrote: > DPRINTF() calls will have to be modified since we want to add line number > of the .sm files where the call appears. I think we can maintain the syntax > of DPRINTF() as used in other places. But in order to combine adjacent > DPRINTF() statements, we would need change to the way DPRINTF() call is > processed. As per my understanding, currently SLICC compiler assumes that > only one object / variable would be printed at a time.
I would prefer not to have the .sm DPRINTF calls look exactly like the c++ calls. In particular, it seems unecessary to specify the trace flag "RubySlicc" at every DPRINTF statement. I don't think we'll ever need more than one trace flag for the .sm debug statements. We've never needed more than one in the past. Also by removing the trace flag, it will be one less thing (though I understand it is relatively minor) for the protocol programmer to worry about. > On 2010-10-21 13:35:21, Steve Reinhardt wrote: > > src/mem/SConscript, line 61 > > <http://reviews.m5sim.org/r/277/diff/1/?file=4454#file4454line61> > > > > Do we want to define a whole new disjoint set of trace flags just for > > Ruby? For example, why have RubyCache and Cache both? I don't think we need a whole disjoint set of trace flags and for most new flags, I don't think we need to explicitly identify them as Ruby flags. However, I can see how one would want a separate RubyCache flag since we currently have both a RubyCache and Cache configuration python object. Overall, I'm indifferent and I'm willing to let Nilay use his best judgement here or let others insist on one way or another. - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/277/#review398 ----------------------------------------------------------- On 2010-10-25 13:10:22, Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/277/ > ----------------------------------------------------------- > > (Updated 2010-10-25 13:10:22) > > > Review request for Default and Ruby Reviewers. > > > Summary > ------- > > Ruby currently uses GEMS debug support with the enum character string map to > enable certain debug messages. Meanwhile, M5 has debug print support that > works with scons. Compiling the m5.fast binary, the M5 debug statements are > removed, but the Ruby ones are not unless RUBY_DEBUG is not defined. This > patch moves Ruby to M5's debug print support. > > > Diffs > ----- > > src/cpu/testers/rubytest/CheckTable.cc f166f8bd8818 > src/mem/SConscript f166f8bd8818 > src/mem/protocol/MESI_CMP_directory-L1cache.sm f166f8bd8818 > src/mem/protocol/MESI_CMP_directory-L2cache.sm f166f8bd8818 > src/mem/protocol/MESI_CMP_directory-dir.sm f166f8bd8818 > src/mem/protocol/MI_example-cache.sm f166f8bd8818 > src/mem/protocol/MI_example-dir.sm f166f8bd8818 > src/mem/protocol/MOESI_CMP_directory-L1cache.sm f166f8bd8818 > src/mem/protocol/MOESI_CMP_directory-L2cache.sm f166f8bd8818 > src/mem/protocol/MOESI_CMP_directory-dir.sm f166f8bd8818 > src/mem/protocol/MOESI_CMP_directory-perfectDir.sm f166f8bd8818 > src/mem/protocol/MOESI_CMP_token-L1cache.sm f166f8bd8818 > src/mem/protocol/MOESI_CMP_token-L2cache.sm f166f8bd8818 > src/mem/protocol/MOESI_CMP_token-dir.sm f166f8bd8818 > src/mem/protocol/MOESI_hammer-cache.sm f166f8bd8818 > src/mem/protocol/MOESI_hammer-dir.sm f166f8bd8818 > src/mem/ruby/SConsopts f166f8bd8818 > src/mem/ruby/buffers/MessageBuffer.cc f166f8bd8818 > src/mem/ruby/common/Debug.hh f166f8bd8818 > src/mem/ruby/common/Debug.cc f166f8bd8818 > src/mem/ruby/common/NetDest.hh f166f8bd8818 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc > f166f8bd8818 > src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc f166f8bd8818 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc > f166f8bd8818 > src/mem/ruby/network/garnet/flexible-pipeline/Router.cc f166f8bd8818 > src/mem/ruby/network/simple/PerfectSwitch.cc f166f8bd8818 > src/mem/ruby/network/simple/Throttle.cc f166f8bd8818 > src/mem/ruby/network/simple/Topology.cc f166f8bd8818 > src/mem/ruby/storebuffer/storebuffer.cc f166f8bd8818 > src/mem/ruby/system/CacheMemory.cc f166f8bd8818 > src/mem/ruby/system/DirectoryMemory.cc f166f8bd8818 > src/mem/ruby/system/SConscript f166f8bd8818 > src/mem/ruby/system/SparseMemory.cc f166f8bd8818 > src/mem/ruby/tester/RaceyPseudoThread.cc f166f8bd8818 > src/mem/slicc/ast/FuncCallExprAST.py f166f8bd8818 > src/mem/slicc/symbols/StateMachine.py f166f8bd8818 > > Diff: http://reviews.m5sim.org/r/277/diff > > > Testing > ------- > > > Thanks, > > Nilay > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
