> 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.
- 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 - Joel ----------------------------------------------------------- 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
