> On 2012-01-01 16:51:13, Ali Saidi wrote: > > I've got a couple of issues with this change. The first is to do with the > > general direction. You're solving the problem here by removing the > > communication delay from the interrupt pending signal to the processing of > > the interrupt. In CPUs there really is a delay for this to propagate, so > > it's not clear to me that we want to get rid of it. Furthermore, once the > > variables you've added are set, I don't see a way for them to be cleared. > > With the previous code the time buffers clear them, but with the new code > > they're only ever set, so it seems like this couldn't work. Finally, your > > commit message needs to have information about when this problem shows up, > > how you addressed it, and what you believe is now fixed. > > > > Thanks, > > Ali > > > > Nilay Vaish wrote: > Ali, is there a particular reason that the commit stage of processor > would transmit this information of pending interrupt to the fetch > stage? I admit I don't know how this is done in a processor. Is there > not a common line which any stage can read from, and not that a > particular stage informs others that an interrupt is pending? > > In case of clear interrupt, I agree with you that there should > be a delay between the processing of the interrupt and this > information being received by the fetch stage.
There are a number of things that are off with change and, in my opinion, how O3 is handling interrupts. First, as I've brought up in the past, I think making commit decide to accept an interrupt, passing that back to fetch, and trusting fetch to turn off the tap while the interrupt should still be recognized, possibly stepping over a small window intended to allow interrupts, is overly complicated and not likely to work correctly all the time except maybe with Alpha where special circumstances apply. I think we should get rid of that entirely, let commit decide when interrupts happen with the existing ISA specific objects, and if one should happen nuke the pipe like a fault. If a macroop has partially executed, none of its microops are necessarily in flight, although most of the time some would be. O3 checking that the instList is empty, if it is indeed doing that, sounds broken. There should be exactly two conditions for recognizing and processing an interrupt, did the last instruction have isDelayedCommit cleared, and did the interrupts object say that by the rules defined by the ISA (interrupt enable bits, priority levels, whether an interrupt was signaled and how, etc.). Whether instructions are in flight should be irrelevant, although there could be something wonky in place right now to make the system of sending a signal to fetch work. Executing the last microop is not necessary or sufficient to recognize an interrupt. Some instructions in x86 have an interrupt shadow and suppress interrupts immediately after them, beyond the end of the macroop, and some (string instructions) can be interrupted at certain points while they execute. The instructions are supposed to look like they repeat over a string of data, but really they're implemented as a loop in microcode. isDelayedCommit is set when interrupts are not allowed after an instruction and cleared when they are and is highly correlated with isLastMicroop but not interchangeable with it. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/960/#review1792 ----------------------------------------------------------- On 2011-12-30 16:31:40, Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/960/ > ----------------------------------------------------------- > > (Updated 2011-12-30 16:31:40) > > > Review request for Default. > > > Summary > ------- > > O3 CPU: Remove interruptPending from fetch > The fetch stage currently has a variable for tracking whether interrupts > are pending are not. This variable's value is communicated from the commit > stage. Instead, the variable is being moved to the O3CPU class and will > accessed using the pointer to the CPU. > > > Diffs > ----- > > src/cpu/o3/comm.hh ca98021c3f96 > src/cpu/o3/commit_impl.hh ca98021c3f96 > src/cpu/o3/cpu.hh ca98021c3f96 > src/cpu/o3/cpu.cc ca98021c3f96 > src/cpu/o3/fetch.hh ca98021c3f96 > src/cpu/o3/fetch_impl.hh ca98021c3f96 > > Diff: http://reviews.m5sim.org/r/960/diff > > > Testing > ------- > > > Thanks, > > Nilay > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
