> On Aug. 13, 2015, 5:59 p.m., Brad Beckmann wrote: > > Recently we discarded patch 2786 because it optionally allowed SLICC > > programs to specify whether an object is a pointer or not. Our patch was > > motivated because PerfectCacheMemory differed from CacheMemory and was a > > pretty special case. We discarded that patch because it was correctly > > argued that SLICC has a long-standing standard not to expose pointers to > > the SLICC programmer. > > > > This patch would require all SLICC programs to consider pointers. That > > seems far more dangerous than our previous patch. If we are going to keep > > to a standard, then we must keep to it and this patch should be discarded. > > Nilay Vaish wrote: > Brad, I think the file src/mem/slicc/ast/ObjDeclAST.py has problem. It > is not possible to use a variable > that appears in the base class AbstractController without adding some > special code to that file. > > Tell me how will you fix this problem. > > Joel Hestness wrote: > Hmmm, it doesn't seem like this patch exposes the programmer to handling > pointers, but rather allows the programmer to specify controller data members > that should be heap allocated. The action code still appears to treat all > variables as references, which (I think) is our primary aim with the SLICC > pointer-handling standard. Is that a valid assessment? > > The only concern I would have is whether this causes confusion for > programmers familiar with pointers and references, if, for example, they > think the '*' designates that it should be used as a pointer in action blocks > (can SLICC code even include a '->' derefence?). If there could be confusion, > should we consider another way of designating heap allocated variables > (rather than '*')? > > Nilay Vaish wrote: > On Thu, 13 Aug 2015, Joel Hestness wrote: > > Hmmm, it doesn't seem like this patch exposes the programmer to > handling > > pointers, but rather allows the programmer to specify controller data > > members that should be heap allocated. The action code still appears to > > treat all variables as references, which (I think) is our primary aim > > with the SLICC pointer-handling standard. Is that a valid assessment? > > Yes. > > > The only concern I would have is whether this causes confusion for > > programmers familiar with pointers and references, if, for example, they > > think the '*' designates that it should be used as a pointer in action > > blocks (can SLICC code even include a '->' derefence?). If there could > > be confusion, should we consider another way of designating heap > > allocated variables (rather than '*')? > > > > It might. I would not worry since SLICC compiler would not allow code > using '->'. > I would prefer sticking to c/c++ way for declaring pointers. > > Brad Beckmann wrote: > Several thoughts: > > - Thank you for the further explanation. I think I understand this patch > much better now. > - This is a huge change to eliminate essentially 3 special cases in > OjbDeclAST.py. The amount of downstream work is significant and it exposes > memory management to the sm files. Why is it a problem that using member > variables in the AbstractController requires changes to ObjDeclAST.py? The > AbstractContoller should not change very often. > - The fact that these heap objects are not really c/c++ pointers makes it > really confusing to use c/c++ notation. > > Nilay Vaish wrote: > * My major concern is language design. The language should be able to > support statically and dynamically allocated variables seemlessly. As of now > this is not possible. > > * The changes required are minimal. As you can see, very few (mostly one > or two) lines change per file. Your protocols may require slightly more, may > be five lines per file. And you only need to add a pointer, nothing else. > > * There is no new memory that is being allocated. Allocated memory will > only go down since we can now have variables that are not dynamically > allocated. Earlier, even integer variables were dynamically allocated. > Secondly, this memory is allocated only once in the init() function. So the > programmer need not worry about any additional memory being allocated once > the actual simulation starts. > > Joel Hestness wrote: > - This member declaration scheme is also consistent with machine > parameter declarations (e.g. "Sequencer * sequencer;", "MessageBuffer * > requestFromCache,...;") > - It removes the special-case handling of machineID/clusterID, and > primitive vs. non-primitive types in favor of actually knowing whether a > variable is a pointer or not. I can imagine this leading to clearer code > going forward > - This also moves in a direction that provides SLICC more flexibility to > optimize under the covers > > Brad Beckmann wrote: > In the case of machine parameters, I thought the those were actually > pointers? Also they are actually from c++/swig, so we have to use C++ > notation. The heap objects used in this patch are completely encapsulated > within the Machine. The SLICC language purposely wants to avoid having the > programmer care about whether these objects are pointers or not.
This patch is not introducing pointers. Pointers are already part of the language. The protocol author will have to care which parameters are pointers, which are not. With this patch, the same will hold true for objects declared within a machine. As Joel points out, this brings in consistency. The language wants to avoid having the programmer allocate / deallocate memory. That still holds true. The changes required are minimal. As you can see, very few (mostly one or two) lines change per file. Your protocols may require slightly more, may be five lines per file. And you only need to add a pointer, nothing else. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3002/#review6937 ----------------------------------------------------------- On Aug. 3, 2015, 12:15 a.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3002/ > ----------------------------------------------------------- > > (Updated Aug. 3, 2015, 12:15 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11017:b37918d5063f > --------------------------- > ruby: slicc: fix naming of variables > > The way SLICC compiler has been written, it was difficult to use a variable > that appears in the base class AbstractController without adding some special > code to the file src/mem/slicc/ast/ObjDeclAST.py. This eliminates the need > for > any such special code. The problem ultimately turned out to be the way SLICC > handles pointer and non-pointer variables. Till now, SLICC tried to handle > everything using pointers, unless some special rules were specified, as was > being done for three variables: version, machineID and clusterID. Now the Var > class will have a field that indicates whether a variable is pointer or not. > This field is used to differentiate between pointer variables and other > variables. Pointer-based variables are allocated in the init() function of > the > controller, as was the case before. Other variables now need not be pointer > based. Variables that need the constructor be called on them will now > necessarily be declared as pointers. The protocols currently in gem5 have > been > changed accordingly. > > > Diffs > ----- > > src/mem/slicc/ast/InPortDeclAST.py a618349a7953 > src/mem/slicc/ast/LocalVariableAST.py a618349a7953 > src/mem/protocol/MOESI_CMP_directory-dma.sm a618349a7953 > src/mem/protocol/MOESI_CMP_token-L1cache.sm a618349a7953 > src/mem/protocol/MOESI_CMP_token-L2cache.sm a618349a7953 > src/mem/protocol/MOESI_CMP_token-dir.sm a618349a7953 > src/mem/protocol/MOESI_hammer-cache.sm a618349a7953 > src/mem/protocol/MOESI_hammer-dir.sm a618349a7953 > src/mem/slicc/ast/ActionDeclAST.py a618349a7953 > src/mem/slicc/ast/EnqueueStatementAST.py a618349a7953 > src/mem/slicc/ast/FormalParamAST.py a618349a7953 > src/mem/protocol/MESI_Three_Level-L0cache.sm a618349a7953 > src/mem/protocol/MESI_Three_Level-L1cache.sm a618349a7953 > src/mem/protocol/MESI_Two_Level-L1cache.sm a618349a7953 > src/mem/protocol/MESI_Two_Level-L2cache.sm a618349a7953 > src/mem/protocol/MESI_Two_Level-dir.sm a618349a7953 > src/mem/protocol/MI_example-cache.sm a618349a7953 > src/mem/protocol/MI_example-dir.sm a618349a7953 > src/mem/protocol/MOESI_CMP_directory-L1cache.sm a618349a7953 > src/mem/protocol/MOESI_CMP_directory-L2cache.sm a618349a7953 > src/mem/protocol/MOESI_CMP_directory-dir.sm a618349a7953 > src/mem/slicc/ast/ObjDeclAST.py a618349a7953 > src/mem/slicc/ast/OutPortDeclAST.py a618349a7953 > src/mem/slicc/ast/PeekStatementAST.py a618349a7953 > src/mem/slicc/symbols/StateMachine.py a618349a7953 > src/mem/slicc/symbols/Type.py a618349a7953 > src/mem/slicc/symbols/Var.py a618349a7953 > > Diff: http://reviews.gem5.org/r/3002/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
