-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3002/#review7122
-----------------------------------------------------------


So there has been a lot of discussion on this patch, including splitting the 
patch up, but I have not seen a new version of the patch reposted.

Again I really dislike these patches which touch all the .sm files.  The SLICC 
language should be stable...it is 16 years old!  I support adding new SLICC 
features when needed, but I dislike breaking backwards compability.  I feel 
that the work we do at AMD strives not to break backwards compability in the sm 
files.  Changing all the sm files to remove three minor 2-line hacks does not 
seem like the right cost/benefit tradeoff.

We just had one person on our team spend 2+ weeks conforming our protocols to 
Joel's MessageBuffer to SimObjects patch.  We still have a lot of work to 
handle Nilay's Address to Addr patch.  It is not as simple has just running a 
sed command across all of our protocol files.

How about we conpromise.  Can you hold off on checking in any more changes to 
SLICC, including this patch, until we check in our GPU model and associated 
protocol files?  Then you can make these changes across all the available 
protocols.

- Brad Beckmann


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

Reply via email to