Hi Nilay,
I've been talking with Brad here at work about some of these things so I
will finally jump into the conversation via email. First, great job on this
- this has clearly been a substantial amount of work. I'm impressed.
I've got some comments below.
On Tue, Jan 11, 2011 at 3:46 PM, Nilay Vaish <ni...@cs.wisc.edu> wrote:
> Brad, my comments are inline.
>
>
> On Tue, 11 Jan 2011, Beckmann, Brad wrote:
>
> Sure, using a local variable to further reduce the calls to
>>>> getCacheEntry is a great idea. I think that is orthogonal to the
>>>> suggestion I was making. I just want the ability to directly set the
>>>> cache_entry and tbe_entry variables in the trigger function. That way
>>>> the address, cache_entry, and tbe_entry variables are dealt with
>>>> consistently and it avoids adding the separate calls to
>>>> set_cache_entry() and set_tbe () in the inports.
>>>>
>>>
>>> Firstly, we have to set cache and transaction buffer entry variables
>>> whenever
>>> we do allocation or deallocation of entries. This means these calls
>>> cannot be
>>> completely avoided. Secondly, while processing events from the mandatory
>>> queue (as it is called in the current implementations), if these
>>> variables are
>>> not set, we will have to revert to the earlier approach. This would
>>> double the
>>> number of times cache entry lookups are performed as the trigger function
>>> will perform the lookup again. This would also mean that both the
>>> approaches for looking up cache entry in the cache will have to exist
>>> simultaneously.
>>>
>>>
>> Absolutely, we still need the ability to allocate or deallocate entries
>> within actions. I'm not advocating to completely eliminate the set/unset
>> cache and tbe entry functions. I just want to avoid including those calls
>> in the inports. I'm confused why the mandatory queue is different than
>> other queues. They all trigger events in the same way.
>>
>
> if (L1IcacheMemory.isTagPresent(in_msg.LineAddress)) {
> // The tag matches for the L1, so the L1 asks the L2 for it.
> trigger(mandatory_request_type_to_event(in_msg.Type), in_msg.LineAddress);
> }
>
> Brad, mandatory queue is just an example where an inport may perform tag
> lookup before cache and transaction buffer entries has been set. Above is an
> excerpt from the file MOESI_CMP_directory-L1cache.sm. Before the trigger()
> is called, isTagPresent() is called. This means tag look up is being
> performed before cache or transaction buffer entries have been set. Suppose
> the tag was present in L1Icache, then in the trigger() call, we will again
> perform lookup.
>
> Similarly, there is an inport in the Hammer's protocol implementation where
> getCacheEntry() is called before a call to trigger(). Now, why should we use
> getCacheEntry() in the inport and cache entry in the action?
>
>
I'm not sure why having the trigger function implicitly set the cache
entries and tbe entries would require two calls to isTagPresent() if local
variables are involved. Is there some reason why something like the code
below would not be possible?
local_var := getL1ICacheEntry(in_msg.LineAddress)
if (is_valid(local_var))
trigger(Event, in_msg.LineAddress, local_var);
I don't see how having the entries being passed to the trigger function is
functionally different than having to explicitly set things using two
function calls is different - as long as there is a local variable involved
to carry the value across some in_port logic it seems much cleaner and more
consistent to eliminate the need to explicitly call two functions for every
in_port.
I just saw Brad's latest email go by with pseudocode and I think that is
both clean and flexible/functional - in certain cases you can explicitly
make the getCacheEntry call you want, depending on your protocol, in the
call to the trigger function, and in others you can set entries beforehand
into local variables and do arbitrary logic, then pass whatever entry you
want to the trigger function.
Lisa
_______________________________________________
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
