The skid buffer doesn't really have anything to do with it. Instructions are on the wire from fetch when the squash comes. Because they aren't actually in one stage or another they keep coming. By the time they arrive, decode isn't processing any more instructions since it's waiting for the squash to finish. It still moves those from the wire into the list, though. Then it asserts there isn't anything in the list because it should have been processed or put in the skid bufferh, but those squashed instructions are there since the stage is just sort of hanging out. They don't really matter since once decode *does* start processing instructions it will throw them away, but until it does they'll just sit there tripping that assert.
Gabe On 08/14/11 20:00, Korey Sewell wrote: > I think I've tried to explain what that code is supposed to do and why your > code may trigger that assert. > > Maybe I didn't do a good job of that. > > Without hard guidelines to what structures do, O3 will continue to be > confusing. Its my interpretation (at least currently) that all instructions > in "insts" must be processed that cycle whether that is squashed, buffered, > or processed. If that condition is violated then that's violating the > semantics of that code and althought may work in that instance, will cause > further confusion. > > If we don't want that to be the case, I'd suggest we'd merge skidBuffer and > the insts lists. Unfortunately, it's hard for me to evaluate a particular > situation in code without being in the middle of your changeset or having > some type of trace that clearly shows what's going on. > > Again, apologize if I'm being ambiguous or confusing. I still think that > assert is valid though. > > On Sun, Aug 14, 2011 at 10:38 PM, Gabe Black <[email protected]> wrote: > >> 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 >> > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
