> On July 24, 2015, 3:13 p.m., Brad Beckmann wrote: > > I'm surprised you moved forward on this patch. I thought I had > > communicated over email a few weeks ago that we should not go down this > > path before a lot of careful thought and debate on whether this is a good > > thing to do. Personally I think this patch removes many of the benefits of > > having a SLICC ast and goes against many of the main design principles of > > SLICC. SLICC is designed to avoid having the programmer worry about memory > > management. SLICC is a layered development approach where the sm file is > > only meant to considering the protocol logic. I believe that is why SLICC > > has been so successful over the past 16+ years. It allows one to create > > differnet protocols without requiring one to write C++ and handle > > complicated memory management. This patch removes those benefits. > > > > Please do not check this in. We need to come to a consensus on whether > > this is even a good direction to go in first.
On 7/26 Nilay wrote "ruby uses shared pointers for Message objects. But messages are not shared by multiple objects. At a given time, only one object holds the pointer to a message object. So there should not be any need for messages to be shared pointers. They can be deleted as soon as the current owner realizes that the message is no longer required. In my opinion, the question memory management does not arise at all." The next patch you introduce (2989) builds on this patch and introduces new calls to each action that creates a message. That sure seems like to me that these series of patches are bringing up the issues and questions of memory management to the sm programmer. On 7/26 Nilay wrote "I don't know how to measure the success of a language, more so when it is allowed to undergo changes. Comparing the ISA description language and SLICC, I find that SLICC has was change 33% times more even though it has been around for only half the time. To me this means that SLICC is not as stable as it can be. And I think the major reason is that it attempts to do things that are better done in plain C++." SLICC was created by Milo back in 1999. I'm not positive, but the ISA description generator was created by Steve and/or Nate in the early 2000's. I believe SLICC predates the ISA generator. Steve has noted to me that the main reason the ISA generator did not create a full-blown AST like SLICC, is simply because they didn't want to put in all that work. If work was not an issue, I think that the ISA parser could benefit from a full AST like SLICC. Milo put in a lot of work 16 years ago and we've been building and benefiting from that ever since. There is more to SLICC from a language prespective and new research protocols evolve faster than ISAs. I think that is why you see more change in SLICC than the ISA generator. Instead of exposing C++ memory management to sm programmers, how about we consider new and improve language improvements to handle message creation. Perhaps we can introduce a new enqueue function? On 7/26 Nilay wrote "shared_pointers have a cost. I think the savings made is substantial enough to justify the required changes to current protocols. My back of the envelope calculations tell me that for a research group like AMD Research, it makes economic sense as well." The SLICC language does not require shared pointers and for many years SLICC simply generated raw C++ pointers. We should be able to elimiate shared pointers without impacting the protocol sm files. Also what back of the envelope calculations make you think this change makes economic sense to AMD? The economics of incorporating this patch and 2889 is one of the main reasons I'm against it. Rewriting and verifying all actions in our internal and soon to be public protocols would require $10k's of work. - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2988/#review6821 ----------------------------------------------------------- On July 24, 2015, 4:45 a.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2988/ > ----------------------------------------------------------- > > (Updated July 24, 2015, 4:45 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10951:6cc98e435866 > --------------------------- > ruby: slicc: have c++ code in action blocks > > This patch changes the way code in action blocks is specified. Right now > we write SLICC code in action blocks, this patch proposes to have c++ code > instead. This is required for moving away from reference counted messages > currently in use by the ruby memory system. Also, from a longer term > perspective, this is required for writing a coherence protocol for the classic > memory system since most of the code for the protocol would be sourced > from the current c++ code in the classic memory system. > > > Diffs > ----- > > src/mem/protocol/MESI_Two_Level-L1cache.sm 9689ead7b479 > src/mem/protocol/MESI_Two_Level-L2cache.sm 9689ead7b479 > src/mem/protocol/MESI_Two_Level-dir.sm 9689ead7b479 > src/mem/protocol/MESI_Two_Level-dma.sm 9689ead7b479 > src/mem/slicc/ast/ActionDeclAST.py 9689ead7b479 > src/mem/slicc/parser.py 9689ead7b479 > src/mem/slicc/symbols/Action.py 9689ead7b479 > src/mem/slicc/symbols/StateMachine.py 9689ead7b479 > src/mem/slicc/symbols/Transition.py 9689ead7b479 > > Diff: http://reviews.gem5.org/r/2988/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
