----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3160/#review7419 -----------------------------------------------------------
Apologies for not including these in the first request, but I realized a couple other things for this patch: First, can you update the patch description to be clearer about the motivation for the change? Something like: " The testAnd*() functions perform operations on data in a DataBlock. Analogous to the memory access() and SubBlock merge() functions, these pass a data container (e.g. packet or DataBlock) to access the data in the block, and in general, they are the interface to DataBlock handling. For consistency with memories and SubBlock, these functions should be members of the DataBlock class, so move them there. " Then, if we need to keep the Packet* -> PacketPtr changes, can you please more clearly describe what requires those (i.e. SLICC limitations + the call chain through functional*() functions)? src/mem/ruby/common/DataBlock.hh (line 61) <http://reviews.gem5.org/r/3160/#comment6264> It appears that getByte() and setByte() are now only used by the testAnd*() functions and the SubBlock merge() functions. If so, can you please declare getByte() and setByte() as private, and declare SubBlock as a friend class in DataBlock? This will disallow protocols from inappropriately handling DataBlock data (i.e. they should use the testAnd*() interface). - Joel Hestness On Oct. 22, 2015, 11:42 p.m., Brandon Potter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3160/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2015, 11:42 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11175:4519bd6790ee > \-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\-\- > ruby: move testAnd* into DataBlock and change Packet* to PacketPtr > > These changes are motivated by a request from Joel Hestness regarding > http://reviews.gem5.org/r/3113/. The original request was to move the > testAndRead and testAndWrite into DataBlock. The Packet* to PacketPtr change > was necessary to satisfy slicc in "RubySlicc_Exports.sm"; it seems that it's > not possible to specify pointer parameters in slicc "structure" blocks. > Also, the changes to use the typedef are arguably cleaner since it's > consistent across all of the files now. > > > Diffs > ----- > > src/mem/ruby/system/RubySystem.hh 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/simple/Switch.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/slicc_interface/AbstractController.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/slicc_interface/Message.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/slicc_interface/RubyRequest.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/slicc_interface/RubyRequest.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/slicc_interface/RubySlicc_Util.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/structures/RubyMemoryControl.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/structures/RubyMemoryControl.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/system/CacheRecorder.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/simple/SimpleNetwork.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/simple/Switch.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkLink.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/flexible-pipeline/Router.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/flexible-pipeline/Router.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/flexible-pipeline/flit.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/flexible-pipeline/flit.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/flexible-pipeline/flitBuffer.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/flexible-pipeline/flitBuffer.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/simple/SimpleNetwork.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_hammer-dir.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_hammer-dma.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_hammer-msg.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/Network_test-cache.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/Network_test-dir.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/Network_test-msg.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/RubySlicc_Defines.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/RubySlicc_Exports.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/RubySlicc_MemControl.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/common/DataBlock.hh 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/common/DataBlock.cc 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/MessageBuffer.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/MessageBuffer.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/Network.hh 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/GarnetNetwork_d.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/InputUnit_d.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/InputUnit_d.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkInterface_d.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkLink_d.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/NetworkLink_d.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/OutputUnit_d.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/OutputUnit_d.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/Router_d.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/Router_d.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/Switch_d.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/VirtualChannel_d.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/VirtualChannel_d.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/flitBuffer_d.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/flitBuffer_d.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/flit_d.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/fixed-pipeline/flit_d.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/flexible-pipeline/GarnetNetwork.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/flexible-pipeline/GarnetNetwork.cc > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/ruby/network/garnet/flexible-pipeline/NetworkInterface.hh > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/cache/cache.cc 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/packet.hh 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MESI_Three_Level-L0cache.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MESI_Three_Level-L1cache.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MESI_Three_Level-msg.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MESI_Two_Level-L1cache.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MESI_Two_Level-L2cache.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MESI_Two_Level-dir.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MESI_Two_Level-dma.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MESI_Two_Level-msg.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MI_example-cache.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MI_example-dir.sm 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MI_example-dma.sm 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MI_example-msg.sm 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_CMP_directory-L1cache.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_CMP_directory-L2cache.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_CMP_directory-dir.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_CMP_directory-dma.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_CMP_directory-msg.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_CMP_token-L1cache.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_CMP_token-L2cache.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_CMP_token-dir.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_CMP_token-dma.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_CMP_token-msg.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > src/mem/protocol/MOESI_hammer-cache.sm > 3a4d1b5cd05ceed30dd0f5341fac9bbe41a193d6 > > Diff: http://reviews.gem5.org/r/3160/diff/ > > > Testing > ------- > > > Thanks, > > Brandon Potter > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
