> On Nov. 2, 2015, 8:56 p.m., Joel Hestness wrote: > > src/mem/protocol/RubySlicc_Exports.sm, line 189 > > <http://reviews.gem5.org/r/3177/diff/1/?file=50930#file50930line189> > > > > I don't understand the reason for distinguising _wCC types. Should > > L1Cache/L2Cache types not provide cache coherence? > > > > More generally, if we're going to define these statically, we should > > make sure that they are descriptive and provide a fairly complete set of > > machine type names for existing protocols. Can you please update these and > > provide clearer comments as appropriate? > > Sooraj Puthoor wrote: > I do not know the history behind this change. This is originally a patch > from Nilay (http://reviews.gem5.org/r/2551) and I think Nilay will be the > best person to answer this question. > > Brad Beckmann wrote: > The wCC names have existed for 10+ years. The CC actually stands for > Cache-to-Cache and was used to identify which responses required > cache-to-cache transfers. Since a remote L1 or L2 cache could respond to a > request, there are separate L1Cache_wCC and L2Cache_wCC. > > The existing comments and type descriptions are already fairly long. Do > we really need to expand them? I think you are being pretty unreasonable > here. > > Nilay Vaish wrote: > In my opinion neither 3176 nor 3177 are required. I have seen the patch > on the gpu model and how it uses MachineType in GPUCoalescer. As and when > AMD breaks that patch into smaller pieces, I'll re-write some portions so > that we do not need any statically defined machine types. > > Joel Hestness wrote: > If we can pull in the GPU without static MachineTypes, excellent! That's > what I'd prefer, so we wouldn't need this patch. > > If we're going to reintroduce static MachineTypes with this patch, the > intended meaning of "_wCC" is currently different than implied in the types' > descriptions. This is very confusing. It is very reasonable to ask that these > have more precise/accurate descriptions. > > Brad Beckmann wrote: > Nilay, we will split our gpu baseline patch and are going to use the > standard review-checkin process. Nilay, if you want to propose further > updates to the model, you are welcome to do that after we check them in. We > will be happy to review them and go throught the same standard review-checkin > process. I do not want you rewriting our code before we check it in. > > Joel, the descriptions are precise and accurate. Why are you being such > a pain here? > > Nilay Vaish wrote: > > Nilay, we will split our gpu baseline patch and are going to use the > standard review-checkin process. Nilay, if you want to propose further > updates to the model, you are welcome to do that after we check them in. We > will be happy to review them and go throught the same standard review-checkin > process. I do not want you rewriting our code before we check it in. > > Can you explain to me the point of the review process if you are against > re-writing patches? Why not commit them straight away?
I'm sorry if this is coming across as me being a pain. That's not what I'm intending. I feel that there is clear miscommunication about what "_wCC" refers to in the current code, because the descriptions on the "_wCC" types are ambiguous/inaccurate: There are two pairs of types like this (L1Cache vs. L1Cache_wCC, and L2Cache vs. L2Cache_wCC). The current type descriptions on these pairs appear to imply that the "_wCC" types mean "with Cache Coherence", because this is the only thing different about their descriptions. However, "_wCC" certainly does not mean "with cache coherence", because basically all existing state machines work together to provide cache coherence. The current descriptions confuse me. I'm not sure I understand what you mean by "CC actually stands for Cache-to-Cache and was used to identify which responses required cache-to-cache transfers", but it's clear that what you're trying to describe here more accurate than "with cache coherence" in the current type descriptions. Do you mean that the "_wCC" types are used to describe cache-to-cache messages between state machines of the types without "_wCC" (i.e. you can have L1Cache_wCC messages between machines of type L1Cache)? If so, this seems like misuse of MachineTypes, because these are not types of machines, but rather types of messages between machines. If not, there needs to be clearer explanation of why we have these types. - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3177/#review7444 ----------------------------------------------------------- On Oct. 30, 2015, 9:49 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3177/ > ----------------------------------------------------------- > > (Updated Oct. 30, 2015, 9:49 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11183:4565e2649f8f > --------------------------- > ruby: imported from reviewboard patch 2551 > > filename temp_nilay_machtype.patch > > > Diffs > ----- > > src/mem/slicc/symbols/Type.py 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > src/mem/slicc/symbols/SymbolTable.py > 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > src/mem/slicc/parser.py 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > src/mem/slicc/ast/MachineAST.py 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > src/mem/slicc/ast/DeclListAST.py 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > src/mem/protocol/MESI_Two_Level-L1cache.sm > 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > src/mem/protocol/MESI_Two_Level-L2cache.sm > 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > src/mem/protocol/MESI_Two_Level-dir.sm > 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > src/mem/protocol/MESI_Two_Level-dma.sm > 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > src/mem/protocol/RubySlicc_Exports.sm > 4daf60db14d794e2344a6c86a93bdd8273bc5bb6 > > Diff: http://reviews.gem5.org/r/3177/diff/ > > > Testing > ------- > > > Thanks, > > Tony Gutierrez > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
