cpu, o3: consider split requests for LSQ checksnoop operations

Information
Submitter:	Hongil Yoon
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 10917:331f426b24a7
---------------------------
cpu, o3: consider split requests for LSQ checksnoop operations

This patch enables instructions in LSQ to track two physical addresses for corresponding two split requests. Later, the information is used in checksnoop() to search for/invalidate the corresponding LD instructions.

The current implementation has kept track of only the physical address that is referenced by the first split request. Thus, for checksnoop(), the line accessed by the second request has not been considered, causing potential correctness issues.

Testing Done

booting o3 with the classic-memory model was done without any issues (single core, x86).

src/cpu/base_dyn_inst.hh (Diff revision 1)

Spelling error.

Could you elaborate on when this is a problem? Why is this only affecting o3?

Hongil Yoon July 17, 2015, 10:17 a.m. (July 17, 2015, 10:17 a.m.)

This may affect other CPU models (e.g. minor). I am not familiar with it, so I need to read codes.

Andreas Hansson July 27, 2015, 6:30 a.m. (July 27, 2015, 6:30 a.m.)

What ever test case you used for the o3, could you run it on the other CPUs as well? Thanks

Hongil Yoon July 27, 2015, 1:27 p.m. (July 27, 2015, 1:27 p.m.)

I will check whether the other CPUs, e.g., minor cpu, are affected by this issue.

Hongil Yoon Aug. 3, 2015, 7:46 p.m. (Aug. 3, 2015, 7:46 p.m.)

I think that the concern boils down to whether snoop operations are needed for the load-store queues (LSQ) of a CPU model.  If so, this issue will affect the CPU model.
The snoop operation has not been implemented for the Minor CPU.  However, I think that it is required for invalidation requests from private caches.  That is, the load queue needs to be searched for any matching in-flight loads that were (being) executed but haven't been committed yet. In this process, split requests need to be considered as well.
I couldn't read all the codes carefully, but this is my understanding so far.

Nilay Vaish Sept. 15, 2015, 6:57 a.m. (Sept. 15, 2015, 6:57 a.m.)

Andreas,
Hongil wrote over a month back that MinorCPU might be affected.  Those using MinorCPU regularly should be

able to decide better whether changes are actually required or not.  If you want to ensure parity between

between LSQ for the two cpus, I would rather have them either be objects of the same class or make them inherit 

from some base class.

Testing Done:

~		O3-Ruby-timing-FS test was created, and the regression test was done without any issues. Another patch will include the test.
	~	booting o3 with the classic-memory model was done without any issues (single core, x86).

Description:

~		Changeset 10917:2d9a52b0e167
	~	Changeset 10917:331f426b24a7

		cpu, o3: consider split requests for LSQ checksnoop operations

		This patch enables instructions in LSQ to track two physical addresses for corresponding two split requests. Later, the information is used in checksnoop() to search for/invalidate the corresponding LD instructions.

		The current implementation has kept track of only the physical address that is referenced by the first split request. Thus, for checksnoop(), the line accessed by the second request has not been considered, causing potential correctness issues.

Diff:

Revision 2 (+28 -11)

Show changes

	src/cpu/base_dyn_inst.hh
	src/cpu/base_dyn_inst_impl.hh
	src/cpu/o3/iew_impl.hh
	src/cpu/o3/lsq_unit_impl.hh

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Testing Done:

Description:

Changeset 10917:2d9a52b0e167

Changeset 10917:331f426b24a7

Diff:

Status: Closed (submitted)