cpu: Result refactoring

Information
Submitter:	Rekai Gonzalez Alberquilla
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 11761:58decfdf9c7b
---------------------------
cpu: Result refactoring

The Result union used to collect the result of an instruction is now a
class of its own, with its constructor, and explicit casting methods for
cleanliness.

This is also a stepping stone to have vector registers, and instructions
that produce a vector register as output.

Change-Id: I6f40c11cb5e835d8b11f7804a4e967aff18025b9
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>

Testing Done

Builtin regressions

Issue Summary

Description	From	Last Updated	Status
Why is this now a deque? Seems like you still only push_back and pop_front.	Jason Lowe-Power	Jan. 26, 2017, 3:37 a.m.	Resolved

Description:

~		Changeset 11761:de3395fe97bf
	~	Changeset 11761:e62eb2fb62b4

		cpu: Result refactoring

		The Result union used to collect the result of an instruction is now a
		class of its own, with its constructor, and explicit casting methods for
		cleanliness.

		This is also a stepping stone to have vector registers, and instructions
		that produce a vector register as output.

		Change-Id: I6f40c11cb5e835d8b11f7804a4e967aff18025b9
		Reviewed-by: Andreas Sandberg andreas.sandberg@arm.com

Diff:

Revision 2 (+206 -79)

Show changes

	src/cpu/base_dyn_inst.hh
	src/cpu/inst_res.hh
	src/cpu/checker/cpu.hh
	src/cpu/checker/cpu_impl.hh

Does converting this to a class have any impact on performance?

Rekai Gonzalez Alberquilla Jan. 26, 2017, 5:42 a.m. (Jan. 26, 2017, 5:42 a.m.)

The honest reply is I do not know. The long reply is: I did differential performance analysis between the base of the 5 patches and each of the patches. The 2nd or 3rd had a 2% slowdown, because I changed the rename table to use a hash_map, so it mapped regIds to physRegIds. There used to be an extra patch that reverted that. After applying all of them, test runs had a marginal speedup (as the use of regIds allows for some more static optimisations). Satisfied with that result, I merged the patch that reverted the change into the 'generalize the use of regIds' patch, and I did not look further into it.

If you think individual impact must be assesed, I can do a performance regression.

Jason Lowe-Power Jan. 26, 2017, 9:03 a.m. (Jan. 26, 2017, 9:03 a.m.)

Sounds good. It seemed possible that the added vtable lookup multiple times per instruction could have caused a performance drop. But since you checked, and it doesn't, the compiler must be smarter than me!

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

Why is this now a deque? Seems like you still only push_back and pop_front.

Show all issues

Rekai Gonzalez Alberquilla Jan. 26, 2017, 5:41 a.m. (Jan. 26, 2017, 5:41 a.m.)
```
Because I made a mistake :) Thanks for spotting it!
```

Description:

~		Changeset 11761:e62eb2fb62b4
	~	Changeset 11761:58decfdf9c7b

		cpu: Result refactoring

		The Result union used to collect the result of an instruction is now a
		class of its own, with its constructor, and explicit casting methods for
		cleanliness.

		This is also a stepping stone to have vector registers, and instructions
		that produce a vector register as output.

		Change-Id: I6f40c11cb5e835d8b11f7804a4e967aff18025b9
		Reviewed-by: Andreas Sandberg andreas.sandberg@arm.com

Diff:

Revision 3 (+205 -78)

Show changes

	src/cpu/base_dyn_inst.hh
	src/cpu/inst_res.hh
	src/cpu/checker/cpu.hh
	src/cpu/checker/cpu_impl.hh

Ship It!

You have a pending review.

Review Board 2.0.15

Screenshots

Files

Issue Summary

Description:

Changeset 11761:de3395fe97bf

Changeset 11761:e62eb2fb62b4

Diff:

Description:

Changeset 11761:e62eb2fb62b4

Changeset 11761:58decfdf9c7b

Diff: