cpu: Add store-access operations

Information
Submitter:	Tony Gutierrez
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 11187:e5e740fac1fb
---------------------------
cpu: Add store-access operations

This patch was created by Bihn Pham during his internship at AMD.

This patch adds decoupled store operations that are commonly used by
high-performance processors. The patch modifies the O3 CPU model to issue
StoreAccess operations as soon as possible to acquire exclusive permission for
eventual stores. Later data store operations behave as normal.

In the classic memory model, StoreAccess requests are treated as HardPFExReq
and share the existing prefetch queue. Therefore, they affect performance only
if the prefetcher is enabled in the classic model.

In Ruby, StoreAccess requests are treated as RubyStore with NULL data field.

Testing Done

Issue Summary

Description	From	Last Updated	Status
Do packets need to be deleted when sendStoreAccess() fails (here and line 971)?	Joel Hestness	Nov. 2, 2015, 1:20 p.m.	Open
This name change doesn't appear to be necessary. Can you please remove it, or move it to a separate review ...	Joel Hestness	Nov. 2, 2015, 1:20 p.m.	Open

Overall, this looks alright. I'd prefer that the Invalidate name change is removed from this patch, though.

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

Do packets need to be deleted when sendStoreAccess() fails (here and line 971)?

Show all issues

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

This name change doesn't appear to be necessary. Can you please remove it, or move it to a separate review request?

Show all issues

Nice addition., but this patch does indeed need to be split. There are name changes that should go in a patch on its own.

When it comes to the core functionality, I am curious why we do not simply stick with HardPFExReq. It seems the patch adds two commands, when one should be enough. Also, the HardPFExReq already has NeedsExclusive, so the new attribute also seems unecessary. It would be great if we can get all these issue resolved.

Steve Reinhardt Nov. 11, 2015, 9:39 a.m. (Nov. 11, 2015, 9:39 a.m.)

This is an old old patch (that has been posted before, see http://reviews.gem5.org/r/2277), and got reposted because it was in the GPU model batch. I expect that it predates HardPFExReq. Good catch, we should use that instead now that it's available.

I expect that the Invalidation/Invalidate name change might just be an uninitentional side effect of a merge, and can be eliminated. Just a guess at this point though.

Responding to Joel's point above, there is a memory leak with the classic memory system that needs to be fixed before we commit, so I'm sure we'll be reposting.

You have a pending review.

Review Board 2.0.15

This change has been discarded.

Screenshots

Files

Issue Summary

Status: Discarded