src/mem/request.hh (Diff revision 1)

Perhaps be a bit more verbose to please Doxygen

Show all issues

1. Brad Beckmann April 5, 2012, 5:24 a.m. (April 5, 2012, 5:24 a.m.)

   Done.

Where/when is this used?  It seems kind of odd to add a new method and not add any use cases.  I can see where support for cache-line-crossing accesses could use this, but only if that's done post-translation (otherwise splitOnVaddr() would be sufficient).

Someone going back and trying to clean things up would be tempted to get rid of this as dead code.  If there's a separate patch coming that actually uses this method, I'd prefer to see those patches merged.  If not, something in the comment explaining when/where this is needed would be useful.

src/mem/request.hh (Diff revision 2)

Seems like we should just use the default copy constructor here:
req1 = new Request(*this);
req2 = new Request(*this);

Looks like you just copied splitOnVaddr(), so this issue should be fixed there too.

Show all issues

1. Gabe Black April 6, 2012, 5:08 a.m. (April 6, 2012, 5:08 a.m.)

   And since you just copied splitOnVaddr, you should factor out all the common code into a new helper function and just specialize the little bits that are different.

2. Brad Beckmann April 6, 2012, 8:13 a.m. (April 6, 2012, 8:13 a.m.)

   We actually don't even use this helper function any more, so I'm just going to drop this patch.