> On Sept. 17, 2015, 5:32 p.m., Brad Beckmann wrote:
> > The description of this patch scares me. Is your plan to allocate the data
> > block seperately from the cache entry data structures? Why don't we move
> > all the creation of the cache entires and data blocks to the generated
> > code? We could have SLICC generate the implementation of
> > CacheMemory::init() and avoid having to dynamically create cache entries
> > and data blocks every time the sm files "allocate" a cache entry. Instead
> > CacheMemory::allocate could simply reset the values of the entry. I
> > suspect all those extra allocations costing us performance.
> >
> > By the way, you said "This is because cache entries are allocated in SLICC
> > code and SLICC does not allow defining constructors that can take input
> > arguments". Doesn't SLICC support "constructor_hacks"?
>
> Nilay Vaish wrote:
> > The description of this patch scares me. Is your plan to allocate the
> data block seperately from the cache entry data structures?
>
> I asking for allocating data blocks the first time they are assigned some
> data. The problem here is lack of constructors in SLICC.
>
> > Why don't > we move all the creation of the cache entires and data
> blocks to the generated code? We could have SLICC generate the
> > implementation of
> > CacheMemory::init() and avoid having to dynamically create cache
> entries and data blocks every time the sm files "allocate" a cache entry.
> > Instead CacheMemory::allocate could simply reset the values of the
> entry. I suspect all those extra allocations costing us performance.
>
> I would rather prefer going the other way round which is move everything
> to plain C++.
>
> > By the way, you said "This is because cache entries are allocated in
> SLICC code and SLICC does not allow defining constructors that can take >
> input arguments". Doesn't SLICC support "constructor_hacks"?
>
> There is a "constructor" construct, but that can only be used to provide
> arguments when an object is being constructed.
> So I can write: TBETable table, constructor="num_entries"; This will
> convert to table = new TBETable(num_entries);
>
> But I cannot use this construct to make SLICC output a constructor of my
> choice. So if I declare a type:
> structure TBEEntry
> {
> int id;
> DataBlock d;
> Time t;
> }
>
> As per my understanding, SLICC cannot output a constructor that calls
> user-chosen constructors on the data members.
> SLICC just assumes that each data member has a constructor that does not
> take any arguments.
Why do you want to move everything into plain C++? You and I have a
fundamental disagreement on the direction to take SLICC, which continually
comes up in these reviews. We should resolve that disagreeement before you
spend a lot of effort developing code like this patch. I know you are focused
on improving Ruby performance and that's great, but let's not get rid of SLICC
for the sake of performance. There are other simulators and even other gem5
memory models (Classic) that have been designed to be faster. I do not think
it does the community to do any good to have Ruby turn into the Classic model.
People use Ruby to model coherence protocols. Coherence protocols are
complicated, so SLICC is designed to hide many software implementation details
so that developers can focus on getting the protocol logic correct and
streamlined.
As for your constructor issue, SLICC builds a full AST and provides all the
inherent benefits of code generation. I think we could modify SLICC to
generate the constructors you want and not impact the existing sm files.
- Brad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3119/#review7206
-----------------------------------------------------------
On Sept. 17, 2015, 1:18 a.m., Nilay Vaish wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3119/
> -----------------------------------------------------------
>
> (Updated Sept. 17, 2015, 1:18 a.m.)
>
>
> Review request for Default.
>
>
> Repository: gem5
>
>
> Description
> -------
>
> Changeset 11122:1a3619d669d1
> ---------------------------
> ruby: copy data to TBE only when required
> This patch changes protocols so that data is copied to a TBE only when
> required. This is required since a later patch drops static variable for
> block
> size in RubySystem class. That variable was used for allocating data buffer
> in
> the constructor for the DataBlock class. But now that would not happen.
> Instead a constructor with size argument will be added. Even with that
> constructor, it is not possible to allocate data block to a cache entry at its
> construction time. This is because cache entries are allocated in SLICC code
> and SLICC does not allow defining constructors that can take input arguments.
> So we cannot supply data block size to the constructor for the Cache Entry.
> So, from now on we copy data from cache block to TBE only when we know for
> sure
> that the cache block has a data block.
>
>
> Diffs
> -----
>
> src/mem/protocol/MESI_Three_Level-L0cache.sm 5a2e1b1b5c43
> src/mem/protocol/MESI_Three_Level-L1cache.sm 5a2e1b1b5c43
> src/mem/protocol/MESI_Two_Level-L1cache.sm 5a2e1b1b5c43
> src/mem/protocol/MESI_Two_Level-L2cache.sm 5a2e1b1b5c43
> src/mem/protocol/MOESI_CMP_directory-L1cache.sm 5a2e1b1b5c43
> src/mem/protocol/MOESI_hammer-cache.sm 5a2e1b1b5c43
>
> Diff: http://reviews.gem5.org/r/3119/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Nilay Vaish
>
>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
