mem: Make the requests carried by packets const

Information
Submitter:	Andreas Hansson
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 10570:c19d57302758
---------------------------
mem: Make the requests carried by packets const

This adds a basic level of sanity checking to the packet by ensuring
that a request is not modified once the packet is created. The only
issue that had to be worked around is the relaying of
software-prefetches in the cache. The specific situation is now solved
by first copying the request, and then creating a new packet
accordingly.

Testing Done

Issue Summary

Description	From	Last Updated	Status
Why do we need the allocate() call? Since we know what the request is, can we not perform the allocation ...	Nilay Vaish	Nov. 20, 2014, 9:45 a.m.	Dropped
Someone has to allocate storage for the packet, and we either do that using "static" pointers where the packet carries ...	Andreas Hansson	Nov. 20, 2014, 9:45 a.m.	Dropped

src/mem/cache/cache_impl.hh (Diff revision 1)

Why do we need the allocate() call? Since we know what the request is, can we not perform the allocation in the constructor itself?

Show all issues

Andreas Hansson Nov. 20, 2014, 9:46 a.m. (Nov. 20, 2014, 9:46 a.m.)

Have a look at: http://reviews.gem5.org/r/2499/

The whole ownership issue is rather confusing, and these patches try to make it a bit less so.

src/mem/cache/cache_impl.hh (Diff revision 1)

Someone has to allocate storage for the packet, and we either do that using "static" pointers where the packet carries a pointer around, but is not responsible for the ownership, or "dynamic" where the ownership lies with the packet itself.

We cannot allocate the space in the constructor as the "static" data does not rely on it.

Show all issues

src/mem/cache/cache_impl.hh (Diff revision 1)

Is one vs. two spaces after periods in the gem5 style guide?  :)

src/mem/cache/cache_impl.hh (Diff revision 1)

This part seems kind of ad hoc... why is it that src/dest need to be copied but nothing else?  I believe it's true, it just seems arbitrary with nothing else to go on here.

Also seems like we should have confidence that the packet is created cleanly (the last 4 asserts seem like overkill).

Andreas Hansson Nov. 30, 2014, 1:20 a.m. (Nov. 30, 2014, 1:20 a.m.)
```
Remnants of my debugging. I've removed it and bumped the patch.
```

Description:

~		Changeset 10547:e8cae196bce7
	~	Changeset 10570:c19d57302758

		mem: Make the requests carried by packets const

		This adds a basic level of sanity checking to the packet by ensuring
		that a request is not modified once the packet is created. The only
		issue that had to be worked around is the relaying of
		software-prefetches in the cache. The specific situation is now solved
		by first copying the request, and then creating a new packet
		accordingly.

Diff:

Revision 2 (+20 -42)

Show changes

	src/mem/packet.hh
	src/mem/cache/cache_impl.hh

Ship It!

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Issue Summary

Description:

Changeset 10547:e8cae196bce7

Changeset 10570:c19d57302758

Diff:

Status: Closed (submitted)