> On May 14, 2015, 2:43 p.m., Joel Hestness wrote: > > I agree with Nilay on this one. If there is a reason to have a pointer, > > then the C++ code should export a pointer type. Brad, it seems you also > > agree. From your response on review 2790 ( http://reviews.gem5.org/r/2790/ > > ): > > >SLICC has been designed not to directly expose pointers to the programmer > > > > So, this patch breaks a solid programming abstraction that already exists > > in SLICC. Further, if this patch were introduced and people use getPointer, > > then reverting this change later will be painful. It would be best to fix > > things the right way here. > > Brad Beckmann wrote: > I don't think there is an easy way to avoid this unless one were to > re-implement PerfectCacheMemor and DirectoryMemory. That goes well beyond > this patch and has many implications. This patch has existed for 3+ years > because there is no easy fix here. Woud you be ok if we changed the name of > the function from getPointer() to convertDirEntryToCacheEntry()? > > Brad Beckmann wrote: > On 5/19 Nilay said "No. > > Can you explain how DirectoryMemory and PerfectCacheMemory are different > from CacheMemory? To me the fact that the patch has existed for 3+ years > means that nobody tried to do things the way I feel they should be done. > Please maintain regularity across different structures. I do not want to > treat DirectoryMemory any different from CacheMemory." > > The difference is all three structures have lookup functions that return > different data types. The function getCacheEntry assumes lookup returns a > CacheEntry pointer. > > This is a simple change that helps fix a problem we are having and > doesn't impact other protocols. Please do not protest just because it wasn't > done the way you would have done it. There is only one Nilay and there are > many issues encountered when developing protocols that cannot be solved by a > single person.
Brad, you seem pretty overzealous here: Nilay's not the only one that opposes this change: I do too, and as I pointed out before, so do you. By our powers combined, surely we can solve 3 times as many issues and actually robustify the codebase rather than just tacking on special features. PerfectCacheMemory is the only odd ball. The right fix was trivial and actually cleans up a very old SLICC bug. Please consider this patch instead: http://reviews.gem5.org/r/2862/ - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2786/#review6255 ----------------------------------------------------------- On May 26, 2015, 7:28 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2786/ > ----------------------------------------------------------- > > (Updated May 26, 2015, 7:28 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10816:aebe70c731a7 > --------------------------- > slicc: convert a variable to a pointer address > > Add the getPointer() SLICC function that simply adds an ampersand to the > variable so that it can be assigned to pointer. > > > Diffs > ----- > > src/mem/slicc/ast/GetPointerAST.py PRE-CREATION > src/mem/slicc/ast/__init__.py df2aa91dba5b0f0baa351039f0802baad9ed8f1d > src/mem/slicc/parser.py df2aa91dba5b0f0baa351039f0802baad9ed8f1d > > Diff: http://reviews.gem5.org/r/2786/diff/ > > > Testing > ------- > > > Thanks, > > Tony Gutierrez > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
