cpu: o3: Merging haltContext with suspendContext

Information
Submitter:	Alexandru Dutu
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 10857:5dd0809b9662
---------------------------
cpu: o3: Merging haltContext with suspendContext
This patch improves suspendContext by flushing the pipeline, which frees
resources for other hardware threads. Secondly, it makes haltContext call
suspendContext which is not freeing architectural registers mappings on halt.
Something that haltContext previously did and was not realistic.
For example, in SMT implementations the architectural registers for a
particular hardware thread will always have mapped some physical registers and
having one thread finish execution will never create more available physical
registers for other hardware threads as there will be scheduled a different
software thread to execute on that hardware thread anyway.
As a consequence, this patch helps enabling SMT in x86 by not putting the
physical registered mapped to the ZeroRegister on the freeList for a different
thread to pick up when one of the threads has finished executing and called
exit.

Testing Done

Quick regressions passed.

Issue Summary

Description	From	Last Updated	Status
Can you please set flags consistently for this instruction? Either move this up into the X86MicroopBase constructor call, or move ...	Joel Hestness	May 28, 2015, 8:56 a.m.	Resolved
Is there a reason not to include code to clear stalls within fetch.squash() and decode.squash()? The stalls[tid] arrays are only ...	Joel Hestness	May 28, 2015, 11:51 a.m.	Resolved
Removing this assertion seems to be a symptom of changing the abstraction for communicating stalls between pipe stages.	Joel Hestness	May 29, 2015, 8:26 a.m.	Dropped

Ship It!

Alexandru Dutu May 28, 2015, 11:12 a.m. (May 28, 2015, 11:12 a.m.)
```
Thanks for the insightful comments Joel!
```

src/arch/x86/isa/microops/specop.isa (Diff revision 1)

Can you please set flags consistently for this instruction? Either move this up into the X86MicroopBase constructor call, or move the setFlags bits (IsNonSpeculative, IsQuiesce) down into the body here to be consistent with this line of code? I don't see any other examples of flag setting in micro-op code, but x86 macro-op code seems to follow the latter convention.

Show all issues

Alexandru Dutu May 28, 2015, 11:12 a.m. (May 28, 2015, 11:12 a.m.)

Yeah, having them set in the MicroHalt constructor instead of in the constructor call of X86MicroopBase seems cleaner as well. Thanks for catching this!

src/cpu/o3/cpu.cc (Diff revision 1)

Is there a reason not to include code to clear stalls within fetch.squash() and decode.squash()? The stalls[tid] arrays are only accessed within their respective pipe stages and set/reset based on signals from adjacent pipe stages. Exposing them publicly through the removeStall() functions seems like it breaks the existing pipe stage stall signalling abstraction.

Show all issues

Alexandru Dutu May 28, 2015, 11:12 a.m. (May 28, 2015, 11:12 a.m.)

It seems that there are other stall signals exposed publicly (e.g. drainStall()). The main reason these can not go into the squash method is that there are scenarious when squashes w/ decode and fetchs stall signals enabled seem to be required. This seems a bit unintuitive and haven't digged into why this is happening, however having these stall signals reset in squash causes the pipeline to stop executing at some point.

Alexandru Dutu May 28, 2015, 11:51 a.m. (May 28, 2015, 11:51 a.m.)

Actually, this can be placed in the squash functions. The reason it was not working when I tried it is that decode has two overloaded squash functions and I was reseting the signal just in one of them.

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

Removing this assertion seems to be a symptom of changing the abstraction for communicating stalls between pipe stages.

Show all issues

Alexandru Dutu May 28, 2015, 11:12 a.m. (May 28, 2015, 11:12 a.m.)

Hypothetically, even if DefaultFetch::doSquash() resets the stall signal from decode, there still is a situation when decode is unblocked and there is no need to assert the decode stall signal.

Change Summary:

Solved the first 2 issues Joel raised.

Description:

~		Changeset 10858:2fb10c49dd93
	~	Changeset 10857:5dd0809b9662

		cpu: o3: Merging haltContext with suspendContext
		This patch improves suspendContext by flushing the pipeline, which frees
		resources for other hardware threads. Secondly, it makes haltContext call
		suspendContext which is not freeing architectural registers mappings on halt.
		Something that haltContext previously did and was not realistic.
		For example, in SMT implementations the architectural registers for a
		particular hardware thread will always have mapped some physical registers and
		having one thread finish execution will never create more available physical
		registers for other hardware threads as there will be scheduled a different
		software thread to execute on that hardware thread anyway.
		As a consequence, this patch helps enabling SMT in x86 by not putting the
		physical registered mapped to the ZeroRegister on the freeList for a different
		thread to pick up when one of the threads has finished executing and called
		exit.

Diff:

Revision 2 (+25 -83)

Show changes

	src/arch/x86/isa/microops/specop.isa
	src/cpu/o3/cpu.hh
	src/cpu/o3/cpu.cc
	src/cpu/o3/decode_impl.hh
	src/cpu/o3/fetch_impl.hh

Looks good. Thanks for investigating where to reset the stalls!

You have a pending review.

Review Board 2.0.15

Screenshots

Files

Issue Summary

Change Summary:

Description:

Changeset 10858:2fb10c49dd93

Changeset 10857:5dd0809b9662

Diff: