> On Feb. 1, 2012, 8:39 a.m., Steve Reinhardt wrote: > > src/cpu/o3/commit_impl.hh, line 1004 > > <http://reviews.gem5.org/r/1018/diff/1/?file=21617#file21617line1004> > > > > Why is there a check for tid == 0 here? > > > > Also, where does the Alpha pal-mode check come from? I don't see it in > > the original code. > > > > Nilay Vaish wrote: > Interrupts can only be handled by the thread with tid 0, so this > check is required. I think we can elide this check, if assume that > number of threads will always be set to 1. The Alpha pal-mode check > has been copied from the fetch stage. It might be possible that the > committed microop is marked non-delayed but is still a microop in the > pal-code. In this, case we do not want to handle the interrupt now. > > Steve Reinhardt wrote: > Where does the tid 0 restriction come from? It's probably moot since I > don't know of any ISAs for which we support SMT in FS mode, but it seems odd > that a "new" restriction is being introduced here in commit that wasn't here > previously. Similarly for the Alpha pal-mode check... why is it necessary to > do this in commit now when it wasn't necessary before? >
Given my understanding of the code, there is no new restriction being introduced. Interrupts were always handled using thread 0. We any way need some check to ensure that the last microop of the thread that handles the interrupt was marked non-delayed. It is possible to introduce an array of booleans to maintain per thread flag. But that seems unnecessary if only thread 0 is going to handle the interrupt. The pal-mode check is necessary since interrupts cannot be handled in pal-mode. It was not there before does not mean that what ever was happening before was correct. Even the check for delayed commit was not there before. By this logic, the entire patch would seem unnecessary. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1018/#review2030 ----------------------------------------------------------- On Jan. 28, 2012, 9:01 p.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1018/ > ----------------------------------------------------------- > > (Updated Jan. 28, 2012, 9:01 p.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 8732:75e54d141aa0 > --------------------------- > O3 CPU: Strengthen condition for handling interrupts > The condition for handling interrupts is to check whether or not the cpu's > instruction list is empty. As observed, this can lead to cases in which even > though the instruction list is empty, interrupts are handled when they should > not be. The condition is being strengthened so that interrupts get handled > only > when the last committed microop did not had IsDelayedCommit set. > > > Diffs > ----- > > src/cpu/o3/commit.hh 9d7c1dc54954 > src/cpu/o3/commit_impl.hh 9d7c1dc54954 > src/cpu/o3/fetch_impl.hh 9d7c1dc54954 > > Diff: http://reviews.gem5.org/r/1018/diff/diff > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
