> On Jan. 31, 2012, 10:08 a.m., Ali Saidi wrote:
> > src/cpu/o3/commit_impl.hh, line 1005
> > <http://reviews.gem5.org/r/1018/diff/1/?file=21617#file21617line1005>
> >
> >     Is this changing the way interrupts work to interrupt immediately? That 
> > is ok, but i would be interesting to know that its changing. 
> >     
> >     If this isn't the case, do we care about if the head instruction is 
> > delayed commit because there are more in the pipe.
> >
> 
> Nilay Vaish wrote:
>     What do you mean by immediately? Earlier the interrupts were handled
>     only at the beginning of the commit cycle. Now they can be handled 
>     in the middle of the commit cycle as well. What can happen, and I 
>     have seen this, that there are no instructions in the pipe and the 
>     head instruction was delayed commit. For example, if the head inst 
>     is marked IsSquashAfter, it can empty the pipe, independent of 
>     whether or not it is delayed commit. Hence, we need the check.
> 
> Ali Saidi wrote:
>     What i mean is that currently the (attempted behavior) is that when an 
> interrupt is signaled the CPU is drained of all instructions and the 
> interrupt is handled. It seems like this might be changing things to 
> interrupt as soon as possible based on the delayed commit flag and not drain 
> the pending instructions in the pipe.
> 
> Nilay Vaish wrote:
>     No, the condition for handling interrupts has only been strengthened.
>     The previous condition that cpu's instList be empty has been retained. 
>     This means when an interrupt is handled, the instList is empty and the
>     last committed microop was not marked delayed commit.
> 
> Steve Reinhardt wrote:
>     If I understand Ali's point, the difference he's referring to is that 
> previously handleInterrupt() could only be called at most once per call to 
> commit() (at the beginning), where now handleInterrupt() is inside a loop and 
> so it could be called more "immediately" in the sense that if the conditions 
> for calling handleInterrupt() become true in the middle of the loop it would 
> get called right away rather than waiting to the next call to commit().

Steve, what you are saying is true, but what is also true is that the 
cpu's instList would be empty. That condition has not been removed.

It might be that calling handleInterrupt() once will be sufficient.
I was initially trying not to rely on the fetch stage to stop supplying
instructions when an interrupt needs to be handled. I felt that the 
function handleInterrupt() should be called in the loop, as a non-delayed 
commit microop may never get committed just at the end of the loop. That 
some how did not work, possibly because of other things that need to fixed 
with the fetch stage. These other things might have been fixed with 
another patch that I have posted. But I retained calling handleInterrupt()
in the loop.


- Nilay


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/1018/#review2021
-----------------------------------------------------------


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

Reply via email to