X86: Break flags in to read and write sets

Information
Submitter:	Nilay Vaish
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 8963:eb9b72c1c919
---------------------------
X86: Break flags in to read and write sets
Currently, each instruction specifies the flags it is going to read, write
as a single set. This patch introduces separate read and write sets. This
is required for reducing the RAW dependencies. If no flag bit needs to be
read (empty read set), and all the flag bits are being written, then there
is no need to read the flagbits register. The dependencies will be reduced
further when the ccflagbits register is split into multiple registers.

Testing Done

Issue Summary

Description	From	Last Updated	Status
Why did you make a copy of this class?	Gabe Black	April 21, 2012, 1:18 p.m.	Open
Could use some comments here on what this class is (same for RegOp3 and RegOp4). I realize that's not in ...	Steve Reinhardt	April 22, 2012, 11:09 a.m.	Open

It shouldn't be necessary to specify two different sets of flags. The microops already either read the flags, or write the flags with an implicit read to support partial updating, but they don't ever do both (please tell me if I'm forgetting one that does). In either of those cases, we can automatically figure out what goes in a read set or a write set. If it's a microop that reads flags (conditional, in other words) then the flags are all reads. If it's a microop that writes flags (an add, subtract, etc.) then it writes, and reads anything that isn't completely covered by writes.

Nilay Vaish April 21, 2012, 2:49 p.m. (April 21, 2012, 2:49 p.m.)

There are some instructions that do both. For example Adc reads the carry bit and
then updates the flag bits. I think there a couple more that read some flag bit 
and writes the EZFbit.

Gabe Black April 21, 2012, 9:21 p.m. (April 21, 2012, 9:21 p.m.)

Even when they do both, the optional parts are either all reads or all writes. Adc can optionally *write* the carry flag, but it always reads it.

Nilay Vaish April 22, 2012, 12:08 a.m. (April 22, 2012, 12:08 a.m.)

I am unable to think of any other way to figure out the read and write sets, that is
possible cleaner and/or simpler than what the patch does. If you can outline one, I am
ready to think about it. If does seem better, I'll change the current implementation.

Gabe Black April 22, 2012, 6:42 a.m. (April 22, 2012, 6:42 a.m.)

I think the problem is you're thinking of these as read/write sets. They're not, they're on/off switches. What they control are either reads or writes which happen to be optional, determined by what the microop is doing. The carry flag read is not optional and therefore doesn't need a switch. The reads which allow partial writes are not optional in a user controlled way and are implicit in what writes are turned on, so they also don't need their own on/off switches.

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

Why did you make a copy of this class?

Show all issues

Nilay Vaish April 21, 2012, 2:49 p.m. (April 21, 2012, 2:49 p.m.)

The design that I have in mind, will not have the original class
finally. Essentially the copy will not exist.

Gabe Black April 21, 2012, 9:07 p.m. (April 21, 2012, 9:07 p.m.)

Lets avoid this step in the middle then. It makes it harder to see what's different, and there's all the duplication in the mean time.

Nilay Vaish April 22, 2012, 12:08 a.m. (April 22, 2012, 12:08 a.m.)
```
Ok.
```

Description:

~		Changeset 8963:d6f5887beaf0
	~	Changeset 8963:eb9b72c1c919

		X86: Break flags in to read and write sets
		Currently, each instruction specifies the flags it is going to read, write
		as a single set. This patch introduces separate read and write sets. This
		is required for reducing the RAW dependencies. If no flag bit needs to be
		read (empty read set), and all the flag bits are being written, then there
		is no need to read the flagbits register. The dependencies will be reduced
		further when the ccflagbits register is split into multiple registers.

Diff:

Revision 2 (+124 -78)

Show changes

	src/arch/x86/isa/microops/fpop.isa
	src/arch/x86/isa/microops/regop.isa

src/arch/x86/isa/microops/regop.isa (Diff revision 2)

Could use some comments here on what this class is (same for RegOp3 and RegOp4).  I realize that's not in keeping with the style of this file ;-).

Also, totally outside the scope of this patch, but as a longer-term goal it would be nice to take these really big hunks of python and just put them in .py files and import them indirectly... 

Conversely, it would be nice if the microcode files that are .py files but which each exist solely to define a single really long string constant were redone as .isa files or something similar.

Sorry for the big tangent, but this just reminded me of those thoughts.

Show all issues

Nilay Vaish April 22, 2012, 12:24 p.m. (April 22, 2012, 12:24 p.m.)

I'll comment those classes.

I don't get your metric for deciding between what should be in .isa files,
and what goes into .py files.

You have a pending review.

Review Board 2.0.15

This change has been discarded.

Screenshots

Files

Issue Summary

Description:

Changeset 8963:d6f5887beaf0

Changeset 8963:eb9b72c1c919

Diff:

Status: Discarded