Of course, at no point do I mean to suggest that you aren't also busy. But I was hoping you'd have enough familiarity to start with that you'd be able to say what the scoop was with the scenario I described.
Gabe On 08/14/11 18:45, Gabe Black wrote: > I hadn't because I haven't had time. I think it might be the wrong > approach to try to preserve the condition the assert is checking for no > real reason other than to keep the assert. One way or the other, in the > process of moving things past it in my queue, we lost the particular > circumstances that caused the bug to surface. I still think the assert > is subtly wrong, but I don't have a test case to work with any more. > > Gabe > > On 08/14/11 04:43, Korey Sewell wrote: >> I hadn't but figured you already committed something and I got a bit >> sidetracked. >> >> I had made an original comment on the patch for what I thought was going on >> and why I thought it was a correct assert. >> >> Did you try moving the sortInsts() up like I commented in that patch? I >> think that would solve the problem with that assert and preserve the >> condition. >> >> In general, YES, I think that anything in that "insts" list should be >> unsquashed instructions ready to be processed and if it cant be processed it >> needs to go in the skidBuffer. In the inorder model, i merged the two lists >> together to avoid confusion and simplify the process but for O3 it may be >> more important to differentiate between incoming instructions and "leftover" >> instructions because of the more threads and speculative nature of thing. >> >> On Sun, Aug 14, 2011 at 7:20 AM, Gabe Black <[email protected]> wrote: >> >>> Have you had a chance to look at this yet, Korey? I'm moving patches >>> around it so I can keep going, but I'm (thankfully) getting down to last >>> of the pile. I'm going to need to do something with that one soonish. >>> >>> Gabe >>> >>> On 08/08/11 00:21, Gabe Black wrote: >>>> In the situation I described in my commit message, what happens there >>>> that shouldn't? If that's a legitimate sequence of events then the >>>> assert is wrong, and if not, then please let me know which part and I'll >>>> go from there. I don't really follow what you're saying without really >>>> studying the decode stage, and I unfortunately don't have much time for >>>> that in the near future. If you can point out what's wrong, though, I'll >>>> be happy to fix it. >>>> >>>> Gabe >>>> >>>> On 08/01/11 23:22, Korey Sewell wrote: >>>>> ----------------------------------------------------------- >>>>> This is an automatically generated e-mail. To reply, visit: >>>>> http://reviews.m5sim.org/r/777/#review1451 >>>>> ----------------------------------------------------------- >>>>> >>>>> >>>>> >>>>> src/cpu/o3/decode_impl.hh >>>>> <http://reviews.m5sim.org/r/777/#comment1873> >>>>> >>>>> I believe this is a good comment. >>>>> >>>>> It's enforcing that Decode can only receive instructions one of two >>> ways: >>>>> (1) Through the instruction lists (insts) when the Decode stage is >>> in a "Running" state >>>>> (2) Through the skidBuffer when Decode has been previously blocked >>> in a prior cycle. >>>>> It looks to me like that the "sortInsts() function should really be >>> placed on line 578 so as to ensure that Decode always has a chance to block >>> before assuming it can grab fresh instructions from Fetch, however it works >>> because on line 747 Decode blocks if there isn't enough room in the next >>> stage to pass instructions. Block entails saving the leftover instructions >>> to the skidBuffer and setting the state to "Blocking". Fetch will see that >>> "Blocking" state on the next cycle (in the checkSignalsAndUpdate function) >>> and then not pass any new instructions into Decode thus not triggering that >>> assert. >>>>> If that assert is being triggered I'd assume what I described above >>> is broken. >>>>> - Korey >>>>> >>>>> >>>>> On 2011-07-11 05:02:18, Gabe Black wrote: >>>>>> ----------------------------------------------------------- >>>>>> This is an automatically generated e-mail. To reply, visit: >>>>>> http://reviews.m5sim.org/r/777/ >>>>>> ----------------------------------------------------------- >>>>>> >>>>>> (Updated 2011-07-11 05:02:18) >>>>>> >>>>>> >>>>>> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and >>> Nathan Binkert. >>>>>> Summary >>>>>> ------- >>>>>> >>>>>> O3: Don't assert that nothing is pending in the decoder at the start of >>> a cycle. >>>>>> The decoder clears out incoming instructions from fetch on a squash, >>> but there >>>>>> may be instructions on the wire incoming from fetch which don't get >>> cleared. >>>>>> Because the decoder is considered squashing, it doesn't actually >>> process any >>>>>> more instructions as they come in. The instructions that were on their >>> way >>>>>> would then sit in the "insts" list until the next cycle, tripping qn >>> assert. >>>>>> It's ok for them to be there in that case because the decoder will just >>> ignore >>>>>> them. Because "insts" is a queue and can't be scanned to verify that >>> all >>>>>> instructions left on it are squashed, this change simply gets rid of >>> the check. >>>>>> Diffs >>>>>> ----- >>>>>> >>>>>> src/cpu/o3/decode_impl.hh 9f3fedee88e2 >>>>>> >>>>>> Diff: http://reviews.m5sim.org/r/777/diff >>>>>> >>>>>> >>>>>> Testing >>>>>> ------- >>>>>> >>>>>> >>>>>> Thanks, >>>>>> >>>>>> Gabe >>>>>> >>>>>> >>>>> _______________________________________________ >>>>> gem5-dev mailing list >>>>> [email protected] >>>>> http://m5sim.org/mailman/listinfo/gem5-dev >>>> _______________________________________________ >>>> gem5-dev mailing list >>>> [email protected] >>>> http://m5sim.org/mailman/listinfo/gem5-dev >>> _______________________________________________ >>> gem5-dev mailing list >>> [email protected] >>> http://m5sim.org/mailman/listinfo/gem5-dev >>> >> > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
