> On April 14, 2016, 3:32 p.m., Joel Hestness wrote: > > I'm really not a fan of the way we're trying to do this. To summarize, this > > change distributes address mapping parameters through all of the Ruby > > controllers, just so that they can call a global function that takes those > > as parameters and returns the mapping. > > > > This leads to a few problems. First, we'll have to change all of the > > protocol files, and get the correct parameters to the correct controllers > > in config files. This will be painful for users with protocols outside of > > mainline, because they need to reason about how to calculate correct > > parameters. Second, it's a confusing way to distribute mapping parameters; > > they are only used in calls to mapAddressToRange, which is a global > > function. The fact that mapAddressToRange is global indicates that there a > > number of ways we could organize this, some of which would likely avoid > > spreading parameters throughout protocol files. Finally, this is a very > > fragile way to handle component mapping, so it won't be extensible. What do > > I do if I notice that I need more complicated parameterization of the > > directory mapping function? It looks like I'd have to extend the function > > signature for mapAddressToRange to take more parameters, and then > > distribute more parameters through the relevent protocol files. This sounds > > very disruptive. > > > > I think we should be aiming to implement this by moving mapAddressToRange > > into the RubySystem. Then, we can just give each RubySystem instance the > > number of controllers that it needs to map, and it can configure all of the > > appropriate mapping functions and parameters. The existing mapping > > functions used in protocol files can indirectly call the RubySystem's > > mapping functions as appropriate, so we shouldn't need to change protocol > > files (or if we do, it would be a 1-time change). This also encapsulates > > the mapping functionality in a single location, so modifying mapping > > functions and parameters should be far more extensible.
I think that it might be reasonable to move the component mapping functionality into RubySystem because the controllers already have a handle for RubySystem. However, I have a couple concerns: _"What do I do if I notice that I need more complicated parameterization of the directory mapping function? It looks like I'd have to extend the function signature for mapAddressToRange to take more parameters, and then distribute more parameters through the relevent protocol files. This sounds very disruptive."_ Can you provide an example of what you're suggesting for the directory mapping? I'm not saying that it's impossible/unreasonable, but I am not aware of what you'd want to use as the other mapping methods. Currently, we should be able to support the address interleaving method using mapAddressToRange (which looks like a banked cache/memory) and a direct method where a private cache sends to a downstream private cache. We could extend it further if needed so that we have a private cache send to a second private cache that is banked. I believe that would match the functionality that the classic cache model provides. (Andreas, feel free to correct me if I'm mistaken; I'm not very familiar with classic.) _"Then, we can just give each RubySystem instance the number of controllers that it needs to map, and it can configure all of the appropriate mapping functions and parameters."_ By default, RubySystem does not track the controllers; the controllers were previously tracked in a global. With my first attempt at this problem, I moved that global into RubySystem and the controllers were added into it as they were initialized. The RubySystem was aware of the controllers and could handle queries to with the Machine* functions because they were being routed through the RubySystem into it's controller vector. The controller vector was a 2-D array which consisted of a vector of the MachineTypes each of which held a vector of controllers for that given MachineType. So, it's possible to do as you're suggesting, although it's not as simple as you're making it out to be. _END CONCERNS_ If and only if we find that the mapAddressToRange is not flexible enough, I'm willing to try an alternative that you suggest. First, please look at http://reviews.gem5.org/r/3113/ again. Before I do any more coding on this, I want a clear-cut path that everyone agrees on for a component mapping. I am not going to develop another implementation unless there's total agreement on how the solution looks. Andreas, a more flexible implementation probably won't be using AddrRange. If you really believe that Ruby should use it, you might speak up here. Also, you might have an opinion on why it's sufficient for classic (if I understand everything correctly). - Brandon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3433/#review8222 ----------------------------------------------------------- On April 4, 2016, 11:43 p.m., Brandon Potter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3433/ > ----------------------------------------------------------- > > (Updated April 4, 2016, 11:43 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11435:9b3faddc9c0b > --------------------------- > ruby: improved component mapping > > This patch aims are improving how addresses are mapped to different entities > in the memory system. The main issue was having a static member function in > the DirectoryMemory class and several different globally visible functions > almost doing the same thing. The functions map_Address_to_DirectoryNode, > map_Address_to_Directory, and broadcast() are being dropped. There is no > replacement provided for broadcast(), but one should be able to use the one > already available provided in the NetDest class. The functions > map_Address_to_DirectoryNode and map_Address_to_Directory should be replaced > with mapAddressToRange(). > > > Diffs > ----- > > src/mem/protocol/RubySlicc_Util.sm cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/slicc_interface/RubySlicc_ComponentMapping.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/DirectoryMemory.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/DirectoryMemory.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/DirectoryMemory.py > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/Prefetcher.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_AMD_Base-dir.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_AMD_Base-probeFilter.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_CMP_directory-L2cache.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_CMP_directory-dma.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_CMP_token-L1cache.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_CMP_token-L2cache.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_CMP_token-dir.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_CMP_token-dma.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_hammer-cache.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_hammer-dir.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_hammer-dma.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/Network_test-cache.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/RubySlicc_ComponentMapping.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_AMD_Base-Region-CorePair.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_AMD_Base-Region-dir.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_AMD_Base-RegionBuffer.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_AMD_Base-RegionDir.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MESI_Two_Level-dir.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MESI_Two_Level-dma.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MI_example-cache.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MI_example-dma.sm cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_AMD_Base-CorePair.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_AMD_Base-L3cache.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MESI_Two_Level-L1cache.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MESI_Two_Level-L2cache.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/GPU_VIPER_Region-TCC.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MESI_Three_Level-L1cache.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/GPU_VIPER-TCP.sm cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/GPU_VIPER-SQC.sm cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/GPU_VIPER-TCC.sm cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/GPU_RfO-TCCdir.sm cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/GPU_RfO-TCP.sm cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/GPU_RfO-SQC.sm cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/GPU_RfO-TCC.sm cfad34a15729e1d5e096245f5a80ded6e2c379ca > configs/ruby/MI_example.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > configs/ruby/MOESI_AMD_Base.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > configs/ruby/MOESI_CMP_directory.py > cfad34a15729e1d5e096245f5a80ded6e2c379ca > configs/ruby/MOESI_CMP_token.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > configs/ruby/MOESI_hammer.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > configs/ruby/Network_test.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > configs/ruby/GPU_VIPER_Baseline.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > configs/ruby/GPU_VIPER_Region.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > configs/ruby/MESI_Three_Level.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > configs/ruby/MESI_Two_Level.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > configs/ruby/GPU_RfO.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > configs/ruby/GPU_VIPER.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > > Diff: http://reviews.gem5.org/r/3433/diff/ > > > Testing > ------- > > > Thanks, > > Brandon Potter > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
