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



src/mem/packet_queue.cc (line 143)
<http://reviews.gem5.org/r/3153/#comment6256>

    not that comment grammar is the biggest issue, but this is quite a run-on.  
I think this could also be more descriptive, e.g.:
    
    This belongs in the middle somewhere.  Search from the end to order by 
tick.  However, if force_order is set, also make sure not to re-order in front 
of some existing packet with the same address.



src/mem/packet_queue.cc (line 152)
<http://reviews.gem5.org/r/3153/#comment6255>

    to me the equivalent rewrite of this term makes more intuitive sense... you 
want to insert in timestamp order unless force_order is set and the addresses 
match:
    
        !(force_order && i->pkt->getAddr() == pkt->getAddr())



src/mem/packet_queue.cc (line 156)
<http://reviews.gem5.org/r/3153/#comment6254>

    this assertion seems really redundant since we've done at least one '--i' 
since we initialized i to transmitList.end().  true that it doesn't harm 
anything, but it's noise IMO.


- Steve Reinhardt


On Oct. 19, 2015, 7:23 a.m., Andreas Hansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3153/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2015, 7:23 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 11176:9a28ff09d760
> ---------------------------
> mem: Order packet queue only on matching addresses
> 
> Instead of conservatively enforcing order for all packets, which may
> negatively impact the simulated-system performance, this patch updates
> the packet queue such that it only applies the restriction if there
> are already packets with the same address in the queue.
> 
> The basic need for the order enforcement is due to coherency
> interactions where requests/responses to the same cache line must not
> over-take each other. We rely on the fact that any packet that needs
> order enforcement will have a block-aligned address. Thus, there is no
> need for the queue to know about the cacheline size.
> 
> 
> Diffs
> -----
> 
>   src/mem/packet_queue.hh 3a4d1b5cd05c 
>   src/mem/packet_queue.cc 3a4d1b5cd05c 
> 
> Diff: http://reviews.gem5.org/r/3153/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to