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 > -- - Korey _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
