Yes, I understood this is what you meant.  The point I was trying to make
is I have not examined a trace to see what is actually happening (have you
gotten a trace to examine what's happening with the DataBlk value for this
request?).  After digging in a little further, it appears that this line:
https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm#878
will be allocating a directory entry if it doesn't exist already
(build/.../mem/ruby/protocol/Directory_Controller.cc:2022).  Since the
state is already U in the transition you are highlighting, I'm assuming the
DirectoryEntry already exists.  So the allocation would not need to happen,
and thus they'd be taking whatever data entry is already there (it is
unclear to me what happens if the directory entry is being allocated ...
because then there would be no data block to start with).  It certainly
appears that vd_victim is not treating the request it creates as a
writethrough, which is the only case in the directory right now where it
updates the TBE entry from the incoming message.

This kind of stuff is tricky though, and there may be other assumptions
elsewhere I haven't seen that explain it.  So I'd want someone to look at a
trace and see if the DataBlk is being updated despite my initial read being
that something isn't quite right.  Hence, getting a trace -- and likely
adding some DPRINTFs for the ProtocolTrace flag that print out DataBlk at
the beginning and end of this action would be important.

Brad, do you remember if the data values are truly being copied by Ruby
now?  I believe the functional accesses are no longer separate?  If not,
then it perhaps explains why this potential bug hasn't been caught before
(since it's a benign bug).  If so, then it does seem like this would affect
correctness.

My guess is it would be a copy partial if this change is needed -- in case
the prior level of cache only wrote specific words on the line, you'd only
want to update those words.  I could imagine multiple protocols connecting
to the directory, and thus while any one of them might update all of the
words on the line, to ensure correctness for both protocols that write
specific words and ones that write the entire line.

Matt

On Sat, Oct 23, 2021 at 1:53 PM Sampad Mohapatra <su...@psu.edu> wrote:

> Hi Matt,
>
> The following condition is missing in t_allocateTBE, but the corepair
> sends a message with VicDirty - CoherenceRequestType.
>
> if (in_msg.Type == CoherenceRequestType:VicDirty) {
>   tbe.DataBlk = in_msg.DataBlk;
> }
>
> P.S.: I am not sure whether the complete block should be replaced or just
> partially copied.
>
> Thanks,
> Sampad
>
> On Sat, Oct 23, 2021 at 2:44 PM Matt Sinclair <
> mattdsinclair.w...@gmail.com> wrote:
>
>> (Resending to mailing list)
>>
>> Hi Sampad,
>>
>> There are lines directly below the one I pointed to that do potentially
>> overwrite the data there.  But I am not 100% sure -- Brad and Matt P, CC'd
>> may know better or see something I'm missing.
>>
>> Matt
>>
>> On Sat, Oct 23, 2021 at 1:37 PM Sampad Mohapatra <su...@psu.edu> wrote:
>>
>>> Yes, but the data is coming from the directory and not the incoming
>>> message, which has the actual data.
>>>
>>> Should it not be:
>>> *tbe.DataBlk := in_msg.DataBlk;*
>>>
>>> i.e., store the dirty victim block data in the tbe.
>>>
>>> Thanks,
>>> Sampad
>>>
>>> On Sat, Oct 23, 2021 at 1:00 PM Matt Sinclair <
>>> mattdsinclair.w...@gmail.com> wrote:
>>>
>>>> I am not sure I understand completely what you're getting at, but it
>>>> appears the allocation of the TBE entry does store the data:
>>>> https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm#878
>>>>
>>>> Matt
>>>>
>>>> On Thu, Oct 21, 2021 at 11:08 PM Sampad Mohapatra via gem5-users <
>>>> gem5-users@gem5.org> wrote:
>>>>
>>>>> Hello All,
>>>>>
>>>>> I was looking at the MOESI_AMD_Base-CorePair.sm and
>>>>> MOESI_AMD_Base-dir.sm and am not quite sure if the following sequence of
>>>>> events are correct or not. Can you please verify?
>>>>>
>>>>> /////////////////////////
>>>>> At CorePair -> invokes action "vd_victim", which sends a data block
>>>>> with outgoing message.
>>>>>
>>>>> At Directory -> undergoes "transition(U, VicDirty, BL)" on message
>>>>> reception, but doesn't store the received data block in the generated TBE
>>>>> and the message is popped out/discarded.
>>>>> /////////////////////////
>>>>>
>>>>> Is the above expected behaviour ?
>>>>>
>>>>> Thanks and regards,
>>>>> Sampad Mohapatra
>>>>> _______________________________________________
>>>>> gem5-users mailing list -- gem5-users@gem5.org
>>>>> To unsubscribe send an email to gem5-users-le...@gem5.org
>>>>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
>>>>
>>>>
_______________________________________________
gem5-users mailing list -- gem5-users@gem5.org
To unsubscribe send an email to gem5-users-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to