Any tests to confirm: 1) that it works :), 2) that it does what it is intended to do?

This code also helped me get past some assertion failures. I am not quite sure how the tmp block stuff works and with this patch it seems like we shouldn't really be calling invalidateBlk() on tmp blks at all. Is the reason why this works because tmp blocks don't really belong to any set?

Any comments on the testing? Looks good overall.

provided all the regressions still run it looks fine to me (although I'm not user the regressions actually hit the tempBlock code.

Hi Lena,

This is great, thanks!  Pretty much exactly what I was envisioning.

I've got a handful of very minor things below; I expect you're on your way back to Wisconsin, so if you won't be able to get to them soon, let us know and perhaps someone else can take care of them so we can get this committed (assuming you don't have any objections to what I'm suggesting).

Also, I had one more thought: you've added a number of 'assert(blk != tempBlk);' lines, which is a good idea, but it would be nicer if we could get the same effect without the additional clutter.  One possibility would be to set the 'set' field of tempBlk to -1, then add an assert to check for that in LRU::invalidate().  I'd be interested in what others think of that.

src/mem/cache/blk.hh (Diff revision 2)

isTouched is a bool, so rhs is preferably 'false' rather than '0'

Show all issues

src/mem/cache/cache_impl.hh (Diff revision 2)

Note that we already have
  assert(blk && blk->isValid());
at the top of the function, so assert(blk) is redundant here.

Show all issues

src/mem/cache/cache_impl.hh (Diff revision 2)

See previous comment about assert(blk)

Show all issues

src/mem/cache/cache_impl.hh (Diff revision 2)

I expect this could be folded with the enclosing if (blk), i.e., just make the outer condition
   if (blk && blk->isValid()) {

Show all issues

src/mem/cache/cache_impl.hh (Diff revision 2)

I see what you did here... since tempBlk was previously invalidated, re-invalidating it should be unnecessary.  That seems like a fine optimization, but I think the comment should also be updated to better reflect what's going on here.

Show all issues

src/mem/cache/cache_impl.hh (Diff revision 2)

Note there's an early exit from the function above if blk is NULL, so this assert is also unnecessary.

Show all issues

src/mem/cache/tags/lru.cc (Diff revision 2)

why did we lose the space here?  seems like a typo

Show all issues

Ship It!

src/mem/cache/cache_impl.hh (Diff revision 3)

Added extra white space at the end

Show all issues

src/mem/cache/tags/lru.cc (Diff revision 3)

I think Steve mentioned something about combining these. E.g.:

assert(blk && blk->isValid());

Show all issues

Thanks Lena!

Thanks for making the changes!