seems ok to me.. gabe?

src/cpu/o3/fetch_impl.hh (Diff revision 1)

Should this really be unconditionally set to true? If we at a commit point (if delayedCommit is false) then it's ok to take an interrupt there even if we're squashing. Or am I misunderstanding how this code works?

Show all issues

1. Nilay Vaish Feb. 1, 2012, 7:29 p.m. (Feb. 1, 2012, 7:29 p.m.)

   This flag is true if the last microop fetched was marked delayed commit. If the microops 
   are being squashed, then we need to know wheather the youngest non-squashed microop was 
   marked delayed commit or not. I don't know if it possible to figure out the youngest 
   non-squashed microop from the fetch stage. Setting the flag to true will at least ensure 
   that the interrupts are not handled when they cannot be, though we may also miss out on 
   some opportunities to handle interrupts.

2. Gabe Black Feb. 2, 2012, 6:08 a.m. (Feb. 2, 2012, 6:08 a.m.)

   It sounds like this is a temporary fix and that you're working on a larger change that will either make this unnecessary or let you handle delayedCommit properly. If that's the case, then I'm ok with this change for now.

3. Nilay Vaish Feb. 2, 2012, 6:21 a.m. (Feb. 2, 2012, 6:21 a.m.)

   If every one agrees that fetch stage should not be looking at the interrupt
   signal to decide up on whether or not fetch instructions, I am ready to take
   up this issue and try to eliminate the delayed commit flag altogether. I don't
   think it is possible to eliminate delayed commit otherwise.

4. Steve Reinhardt Feb. 2, 2012, 6:41 a.m. (Feb. 2, 2012, 6:41 a.m.)

   Sounds reasonable to me.  I think in general it feels like we are applying band-aids when the situation calls for a more thorough re-thinking of what's going on.  Part of this feeling is because I'm not fully engaged, but maybe it's accurate anyway.  If moving all of the interrupt processing to commit is the re-thinking that's necessary, then I'm all for it.

src/cpu/o3/fetch_impl.hh (Diff revision 1)

Would it be equivalent to check fetchStatus[tid] == Squashing here?  I know the name of the flag is not perfect, but it seems wrong to check a flag called "delayedCommit" in the fetch stage.  Checking fetchStatus seems more intuitive.

Show all issues

1. Nilay Vaish Feb. 2, 2012, 1:16 a.m. (Feb. 2, 2012, 1:16 a.m.)

   I want to eliminate that flag and in fact the entire business of checking for
   interrupts in the fetch stage. I tried doing it but things did not work out. And 
   debugging the O3 CPU is not all that easy either.
   
   To answer your question, the flag has been put in place to check whether or not
   the previously fetched microop was marked delayed commit. This has nothing to do
   with whether or not fetch stage is squashing instructions.

src/cpu/o3/fetch_impl.hh (Diff revision 1)

If this really is necessary, it at least needs a comment explaining why.