Hi Derek, Here is the patch of my changes. If it looks fine, I will run the regression test again with update_ref=True, send out the final patch, and push in the changes.
Thanks Tushar ---------------------------------------------------------------------------------------------------------------- PATCH: # HG changeset patch # User Tushar Krishna <[email protected]> # Date 1249664750 25200 # Node ID e34c4d587f3882676c723576ba7875b9bab29a75 # Parent 0e78ffeebffd237f6532d3261caeac745e5340eb bug fix for data_msg_size in network/Network.cc diff -r 0e78ffeebffd -r e34c4d587f38 src/mem/protocol/MOESI_CMP_directory-L2cache.sm --- a/src/mem/protocol/MOESI_CMP_directory-L2cache.sm Tue Aug 04 09:37:27 2009 -0700 +++ b/src/mem/protocol/MOESI_CMP_directory-L2cache.sm Fri Aug 07 10:05:50 2009 -0700 @@ -1389,7 +1389,7 @@ action(uu_profileMiss, "\u", desc="Profile the demand miss") { peek(L1requestNetwork_in, RequestMsg) { // AccessModeType not implemented - profile_L2Cache_miss(convertToGenericType(in_msg.Type), in_msg.AccessMode, MessageSizeTypeToInt(in_msg.MessageSize), in_msg.Prefetch, machineIDToNodeID(in_msg.Requestor)); + //profile_L2Cache_miss(convertToGenericType(in_msg.Type), + in_msg.AccessMode, MessageSizeTypeToInt(in_msg.MessageSize), + in_msg.Prefetch, machineIDToNodeID(in_msg.Requestor)); } } diff -r 0e78ffeebffd -r e34c4d587f38 src/mem/protocol/MOESI_CMP_token-L2cache.sm --- a/src/mem/protocol/MOESI_CMP_token-L2cache.sm Tue Aug 04 09:37:27 2009 -0700 +++ b/src/mem/protocol/MOESI_CMP_token-L2cache.sm Fri Aug 07 10:05:50 2009 -0700 @@ -916,7 +916,7 @@ action(uu_profileMiss, "\u", desc="Profile the demand miss") { peek(L1requestNetwork_in, RequestMsg) { // AccessModeType not implemented - profile_L2Cache_miss(convertToGenericType(in_msg.Type), in_msg.AccessMode, MessageSizeTypeToInt(in_msg.MessageSize), in_msg.Prefetch, machineIDToNodeID(in_msg.Requestor)); + //profile_L2Cache_miss(convertToGenericType(in_msg.Type), + in_msg.AccessMode, MessageSizeTypeToInt(in_msg.MessageSize), + in_msg.Prefetch, machineIDToNodeID(in_msg.Requestor)); } } diff -r 0e78ffeebffd -r e34c4d587f38 src/mem/protocol/MSI_MOSI_CMP_directory-L2cache.sm --- a/src/mem/protocol/MSI_MOSI_CMP_directory-L2cache.sm Tue Aug 04 09:37:27 2009 -0700 +++ b/src/mem/protocol/MSI_MOSI_CMP_directory-L2cache.sm Fri Aug 07 10:05:50 2009 -0700 @@ -978,7 +978,7 @@ action(uu_profileMiss, "\u", desc="Profile the demand miss") { peek(L1RequestIntraChipL2Network_in, RequestMsg) { - profile_L2Cache_miss(convertToGenericType(in_msg.Type), in_msg.AccessMode, MessageSizeTypeToInt(in_msg.MessageSize), in_msg.Prefetch, L1CacheMachIDToProcessorNum(in_msg.RequestorMachId)); + //profile_L2Cache_miss(convertToGenericType(in_msg.Type), + in_msg.AccessMode, MessageSizeTypeToInt(in_msg.MessageSize), + in_msg.Prefetch, L1CacheMachIDToProcessorNum(in_msg.RequestorMachId)); } } diff -r 0e78ffeebffd -r e34c4d587f38 src/mem/protocol/RubySlicc_Util.sm --- a/src/mem/protocol/RubySlicc_Util.sm Tue Aug 04 09:37:27 2009 -0700 +++ b/src/mem/protocol/RubySlicc_Util.sm Fri Aug 07 10:05:50 2009 -0700 @@ -37,7 +37,6 @@ NodeID intToID(int nodenum); int IDToInt(NodeID id); int addressToInt(Address addr); -int MessageSizeTypeToInt(MessageSizeType size_type); bool multicast_retry(); int numberOfNodes(); int numberOfL1CachePerChip(); diff -r 0e78ffeebffd -r e34c4d587f38 src/mem/ruby/config/cfg.rb --- a/src/mem/ruby/config/cfg.rb Tue Aug 04 09:37:27 2009 -0700 +++ b/src/mem/ruby/config/cfg.rb Fri Aug 07 10:05:50 2009 -0700 @@ -602,7 +602,7 @@ vec += " buffer_size "+buffer_size.to_s vec += " link_latency "+adaptive_routing.to_s vec += " on_chip_latency "+on_chip_latency.to_s - + vec += " control_msg_size "+control_msg_size.to_s end def printTopology() diff -r 0e78ffeebffd -r e34c4d587f38 src/mem/ruby/config/defaults.rb --- a/src/mem/ruby/config/defaults.rb Tue Aug 04 09:37:27 2009 -0700 +++ b/src/mem/ruby/config/defaults.rb Fri Aug 07 10:05:50 2009 -0700 @@ -82,6 +82,8 @@ # on chip latency default_param :on_chip_latency, Integer, 1 + + default_param :control_msg_size, Integer, 8 end class GarnetNetwork < Network diff -r 0e78ffeebffd -r e34c4d587f38 src/mem/ruby/network/Network.cc --- a/src/mem/ruby/network/Network.cc Tue Aug 04 09:37:27 2009 -0700 +++ b/src/mem/ruby/network/Network.cc Fri Aug 07 10:05:50 2009 -0700 @@ -26,9 +26,44 @@ m_adaptive_routing = (argv[i+1]=="true"); else if (argv[i] == "link_latency") m_link_latency = atoi(argv[i+1].c_str()); + else if (argv[i] == "control_msg_size") + m_control_msg_size = atoi(argv[i+1].c_str()); + } - } + m_data_msg_size = RubySystem::getBlockSizeBytes() + + m_control_msg_size; + assert(m_virtual_networks != 0); assert(m_topology_ptr != NULL); -// printf ("HERE \n"); } + +int Network::MessageSizeType_to_int(MessageSizeType size_type) { + switch(size_type) { + case MessageSizeType_Undefined: + ERROR_MSG("Can't convert Undefined MessageSizeType to integer"); + break; + case MessageSizeType_Control: + case MessageSizeType_Request_Control: + case MessageSizeType_Reissue_Control: + case MessageSizeType_Response_Control: + case MessageSizeType_Writeback_Control: + case MessageSizeType_Forwarded_Control: + case MessageSizeType_Invalidate_Control: + case MessageSizeType_Unblock_Control: + case MessageSizeType_Persistent_Control: + case MessageSizeType_Completion_Control: + return m_control_msg_size; + break; + case MessageSizeType_Data: + case MessageSizeType_Response_Data: + case MessageSizeType_ResponseLocal_Data: + case MessageSizeType_ResponseL2hit_Data: + case MessageSizeType_Writeback_Data: + return m_data_msg_size; + break; + default: + ERROR_MSG("Invalid range for type MessageSizeType"); + break; + } + return 0; +} diff -r 0e78ffeebffd -r e34c4d587f38 src/mem/ruby/network/Network.hh --- a/src/mem/ruby/network/Network.hh Tue Aug 04 09:37:27 2009 -0700 +++ b/src/mem/ruby/network/Network.hh Fri Aug 07 10:05:50 2009 -0700 @@ -71,6 +71,8 @@ int getEndpointBandwidth() { return m_endpoint_bandwidth; } bool getAdaptiveRouting() {return m_adaptive_routing; } int getLinkLatency() { return m_link_latency; } + int MessageSizeType_to_int(MessageSizeType size_type); + // returns the queue requested for the given component virtual MessageBuffer* getToNetQueue(NodeID id, bool ordered, int netNumber) = 0; @@ -107,6 +109,8 @@ Topology* m_topology_ptr; bool m_adaptive_routing; int m_link_latency; + int m_control_msg_size; + int m_data_msg_size; }; // Output operator declaration @@ -123,41 +127,4 @@ return out; } -// Code to map network message size types to an integer number of bytes -const int CONTROL_MESSAGE_SIZE = 8; -const int DATA_MESSAGE_SIZE = (RubySystem::getBlockSizeBytes()+8); - -extern inline -int MessageSizeType_to_int(MessageSizeType size_type) -{ - switch(size_type) { - case MessageSizeType_Undefined: - ERROR_MSG("Can't convert Undefined MessageSizeType to integer"); - break; - case MessageSizeType_Control: - case MessageSizeType_Request_Control: - case MessageSizeType_Reissue_Control: - case MessageSizeType_Response_Control: - case MessageSizeType_Writeback_Control: - case MessageSizeType_Forwarded_Control: - case MessageSizeType_Invalidate_Control: - case MessageSizeType_Unblock_Control: - case MessageSizeType_Persistent_Control: - case MessageSizeType_Completion_Control: - return CONTROL_MESSAGE_SIZE; - break; - case MessageSizeType_Data: - case MessageSizeType_Response_Data: - case MessageSizeType_ResponseLocal_Data: - case MessageSizeType_ResponseL2hit_Data: - case MessageSizeType_Writeback_Data: - return DATA_MESSAGE_SIZE; - break; - default: - ERROR_MSG("Invalid range for type MessageSizeType"); - break; - } - return 0; -} - #endif //NETWORK_H diff -r 0e78ffeebffd -r e34c4d587f38 src/mem/ruby/network/garnet-fixed-pipeline/NetworkInterface_d.cc --- a/src/mem/ruby/network/garnet-fixed-pipeline/NetworkInterface_d.cc Tue Aug 04 09:37:27 2009 -0700 +++ b/src/mem/ruby/network/garnet-fixed-pipeline/NetworkInterface_d.cc Fri Aug 07 10:05:50 2009 -0700 @@ -114,7 +114,7 @@ NetDest net_msg_dest = net_msg_ptr->getInternalDestination(); Vector<NodeID> dest_nodes = net_msg_dest.getAllDest(); // gets all the destinations associated with this message. - int num_flits = (int) ceil((double) MessageSizeType_to_int(net_msg_ptr->getMessageSize())/m_net_ptr->getNetworkConfig()->getFlitSize() ); // Number of flits is dependent on the link bandwidth available. This is expressed in terms of bytes/cycle or the flit size + int num_flits = (int) ceil((double) + m_net_ptr->MessageSizeType_to_int(net_msg_ptr->getMessageSize())/m_net + _ptr->getNetworkConfig()->getFlitSize() ); // Number of flits is + dependent on the link bandwidth available. This is expressed in terms + of bytes/cycle or the flit size for(int ctr = 0; ctr < dest_nodes.size(); ctr++) // loop because we will be converting all multicast messages into unicast messages { diff -r 0e78ffeebffd -r e34c4d587f38 src/mem/ruby/network/garnet-flexible-pipeline/NetworkInterface.cc --- a/src/mem/ruby/network/garnet-flexible-pipeline/NetworkInterface.cc Tue Aug 04 09:37:27 2009 -0700 +++ b/src/mem/ruby/network/garnet-flexible-pipeline/NetworkInterface.cc Fri Aug 07 10:05:50 2009 -0700 @@ -109,7 +109,7 @@ NetworkMessage *net_msg_ptr = dynamic_cast<NetworkMessage*>(msg_ptr.ref()); NetDest net_msg_dest = net_msg_ptr->getInternalDestination(); Vector<NodeID> dest_nodes = net_msg_dest.getAllDest(); // gets all the destinations associated with this message. - int num_flits = (int) ceil((double) MessageSizeType_to_int(net_msg_ptr->getMessageSize())/m_net_ptr->getNetworkConfig()->getFlitSize() ); // Number of flits is dependent on the link bandwidth available. This is expressed in terms of bytes/cycle or the flit size + int num_flits = (int) ceil((double) + m_net_ptr->MessageSizeType_to_int(net_msg_ptr->getMessageSize())/m_net + _ptr->getNetworkConfig()->getFlitSize() ); // Number of flits is + dependent on the link bandwidth available. This is expressed in terms + of bytes/cycle or the flit size for(int ctr = 0; ctr < dest_nodes.size(); ctr++) // loop because we will be converting all multicast messages into unicast messages { diff -r 0e78ffeebffd -r e34c4d587f38 src/mem/ruby/network/simple/Switch.cc --- a/src/mem/ruby/network/simple/Switch.cc Tue Aug 04 09:37:27 2009 -0700 +++ b/src/mem/ruby/network/simple/Switch.cc Fri Aug 07 10:05:50 2009 -0700 @@ -169,7 +169,7 @@ int sum = message_counts[type].sum(); if (sum != 0) { out << " outgoing_messages_switch_" << m_switch_id << "_link_" << link << "_" << type - << ": " << sum << " " << sum * MessageSizeType_to_int(type) + << ": " << sum << " " << sum * + (RubySystem::getNetwork()->MessageSizeType_to_int(type)) << " " << message_counts[type] << " base_latency: " << throttle_ptr->getLatency() << endl; } } diff -r 0e78ffeebffd -r e34c4d587f38 src/mem/ruby/network/simple/Throttle.cc --- a/src/mem/ruby/network/simple/Throttle.cc Tue Aug 04 09:37:27 2009 -0700 +++ b/src/mem/ruby/network/simple/Throttle.cc Fri Aug 07 10:05:50 2009 -0700 @@ -275,8 +275,8 @@ // Artificially increase the size of broadcast messages if (BROADCAST_SCALING > 1) { if (net_msg_ptr->getDestination().isBroadcast()) { - return (MessageSizeType_to_int(net_msg_ptr->getMessageSize()) * MESSAGE_SIZE_MULTIPLIER * BROADCAST_SCALING); + return + (RubySystem::getNetwork()->MessageSizeType_to_int(net_msg_ptr->getMess + ageSize()) * MESSAGE_SIZE_MULTIPLIER * BROADCAST_SCALING); } } - return (MessageSizeType_to_int(net_msg_ptr->getMessageSize()) * MESSAGE_SIZE_MULTIPLIER); + return + (RubySystem::getNetwork()->MessageSizeType_to_int(net_msg_ptr->getMess + ageSize()) * MESSAGE_SIZE_MULTIPLIER); } diff -r 0e78ffeebffd -r e34c4d587f38 src/mem/ruby/slicc_interface/RubySlicc_Util.hh --- a/src/mem/ruby/slicc_interface/RubySlicc_Util.hh Tue Aug 04 09:37:27 2009 -0700 +++ b/src/mem/ruby/slicc_interface/RubySlicc_Util.hh Fri Aug 07 10:05:50 2009 -0700 @@ -106,11 +106,6 @@ return (int) addr.getLineAddress(); } -extern inline int MessageSizeTypeToInt(MessageSizeType size_type) -{ - return MessageSizeType_to_int(size_type); -} - extern inline bool long_enough_ago(Time event) { return ((get_time() - event) > 200); ------------------------------------------------------------------------------------------------- -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Derek Hower Sent: Thursday, August 06, 2009 3:04 PM To: M5 Developer List Subject: Re: [m5-dev] Bug in DATA_MESSAGE_SIZE initialization in Ruby 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 _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
