Steve Reinhardt wrote:
> Thanks, Gabe. It's cool to have this working. Just a few minor things:
>
> - It looks like the two requests are issued to the cache
> sequentially... did you consider issuing them concurrently? I don't
> see a reason why this wouldn't work, though it would be a little more
> complex (since the responses could come back in either order). I'm
> not sure myself which one is preferable, or if it even matters that
> much.
>
I'll give that a try. I'm not sure how hard it will be, but it sounds
doable.
> - Instead of allocating new dynamic memory blocks for the two sub
> packets, why not just give them pointers into the larger block? Then
> you wouldn't have to do any merging after the responses come back for
> loads, or copying for writes.
>
I didn't think of that. I'll change it.
> - It's still the official policy that local variables are lower
> w/underscores while class members are mixed case... sometimes I have
> mixed feelings about that one myself but I've been trying to do a
> better job of sticking with it.
>
I was actually under the impression that the underscore variables went
in python and the camel case went in C++. My impression was that it was
very inconsistent but that camel case was usually used. My vote would be
to just get rid of that rule rather than pretend to keep following it.
Why do we have the two different styles anyway?
> - I know we discussed this before, but given that we're not attempting
> to handle split swaps, does it make sense to pretend we're handling
> split ll/sc operations?
>
>
I'll change that.
> - Would it be possible to condense the code a bit? I think doing
> things a little more lockstep with the two requests would help; also
> I'd think you can assume that both requests end up being the same in
> terms of locked/unlocked etc. Something like this:
>
> Req *first_req = Request(...);
> Req *second_req = Request(...);
> fault = thread->translate(first_req);
> if (fault == NoFault) {
> fault = thread->translate(second_req);
> }
> if (fault != NoFault) {
> delete first_req;
> delete second_req;
> return fault;
> }
>
> if (first_req->isSwap() || second_req->isSwap()) {
> panic(...);
> }
>
> etc.
>
I didn't do things this way since you'll allocate the second request
even if the translating the first request fails and you just throw it
away. I can change it if the other way is preferred.
Gabe
_______________________________________________
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
