On Thu, Aug 6, 2009 at 4:56 PM, Krishna, Tushar<[email protected]> wrote: > Hi Derek, > The reason why these variables could not be standard members of the > Network class was because the function to read these values, > MessageSizeType_to_int(MessageSizeType size_type), is an extern > function. This is because this function was earlier being used by some > protocols while calculating stats. We realized that this is no longer > the case, and this function can be made internal to the Network class. > > I did that. These are my changes: > - A parameter called control_msg_size is defined in defaults.rb (and is > also printed out in the ruby.config file from cfg.rb). > - In the Network::init() function, m_control_msg_size gets set to > control_msg_size, > and m_data_msg_size gets set to RubySystem::getBlockSizeBytes() + > m_control_msg_size > - The MessageSizeType_to_int function is a public member of the Network > class and is used in the following places: > garnet_fixed_pipeline/NetworkInterface_d.cc, > garnet_flexible_pipeline/NetworkInterface_d.cc, simple/Switch.cc, > simple/Throttle.cc > - I removed the declaration of this function from > src/mem/protocol/RubySlicc_Util.sm > > I can check these in if you wish. However, I would need to run the > regression tester before that right? And also update the regression > stats files? > Could you tell me how to do that?
Before pushing, send out a patch using the patchbomb mercurial extension (see hg help email). For the regression, it can be run as: scons build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby Since this is likely to change the stat output, you may need to re-reun the regression after it completes with the option update_ref=True. > > Thanks > Tushar > > > > -----Original Message----- > From: [email protected] [mailto:[email protected]] On > Behalf Of Derek Hower > Sent: Tuesday, August 04, 2009 7:46 PM > To: M5 Developer List > Subject: Re: [m5-dev] Bug in DATA_MESSAGE_SIZE initialization in Ruby > > On Tue, Aug 4, 2009 at 9:18 PM, Krishna, Tushar<[email protected]> > wrote: >> Hi, >> There is a small bug in the initialization of DATA_MESSAGE_SIZE (which > is >> used by the Network). >> >> In src/mem/ruby/network/Network.hh >> there is this line: >> >> const int DATA_MESSAGE_SIZE = (RubySystem::getBlockSizeBytes()+8); >> >> The problem is that RubySystem's init function (which sets the block > size) >> is called *after* Network is created, and hence DATA_MESSAGE_SIZE gets > set >> to (0 + 8). >> > > Good catch. > >> Network has its own init function too which is called later, but >> DATA_MESSAGE_SIZE is not part of that. >> >> Brad and I were discussing some potential ways to solve this issue. >> We can either make DATA_MESSAGE_SIZE a local variable of the Network > class >> and set it during the init. However, this leads to things like > declaring >> static variables, referencing the network pointer from the system > pointer >> etc -> a sort of ugly way to set a constant parameter. >> > > I don't think it's that bad. For one, I'm not sure why it would have > to be static. It could just be a standard member of Network. As for > referencing the Network pointer from System, I don't think that's a > problem, either. The system object has a pointer to all other objects > anyway, and I don't believe that is going to change even as we go > toward the M5-like configuration. In fact, I think the goal is to > eventually remove all globals, System included, so Network and System > would presumably have pointers to each other. > > The way to do it that would fit with the current Ruby configuration > style would be to initialize an m_data_message_size member in init() > based on a parameter passed in the argv vector. To do this you'll > need to add a :data_message_size default_param to the Network class in > defaults.rb, print it's value in Network::argv in cfg.rb, then consume > the value in Network.cc::init. > >> Another possibility is using 64 bytes as the block size here and then > having >> an assertion check in the init function... > > Perhaps that would work for whatever it is you're currently working > on, but definitely not a permanent fix. > >> >> Any suggestions from the GEM5 team? >> >> Thanks >> Tushar >> >> _______________________________________________ >> m5-dev mailing list >> [email protected] >> http://m5sim.org/mailman/listinfo/m5-dev >> >> > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > > > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
