base: Encapsulate the underlying fields in AddrRange

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

Description

Changeset 9325:dd82368c5370
---------------------------
base: Encapsulate the underlying fields in AddrRange

This patch makes the start and end address private in a move to
prevent direct manipulation and matching of ranges based on these
fields. This is done so that a transition to ranges with interleaving
support is possible.

As a result of hiding the start and end, a number of member functions
are needed to perform the comparisons and manipulations that
previously took place directly on the members. An accessor function is
provided for the start address, and functions are added to merge two
ranges, and test if an address is within a range. As a result of the
latter the != and == operator is also removed in favour of the member
function. A member function that returns a string representation is
also created to allow debug printing.

In general, this patch does not add any functionality, but it does
take us closer to a situation where interleaving (and more cleverness)
can be added under the bonnet without exposing it to the user. More on
that in a later patch.

Testing Done

util/regress all passing (disregarding t1000 and eio)

Issue Summary

Description	From	Last Updated	Status
Why this change in defintion of intersection? In fact, even the original definition was incorrect.	Nilay Vaish	Oct. 31, 2012, 6:29 p.m.	Dropped

src/base/addr_range.hh (Diff revision 1)

Thanks for doing this. I certainly believe that this increases the readability of the code. Just one comment. I would prefer range.containsAddr(addr) compared to range.inRange(addr) as the later sounds as if we are testing if the addr contains the range. But I'll let you make that choice.

Andreas Hansson Oct. 31, 2012, 4:13 a.m. (Oct. 31, 2012, 4:13 a.m.)
```
Sure, might shorten it to "contains".
```

Nilay Vaish Oct. 31, 2012, 6:06 a.m. (Oct. 31, 2012, 6:06 a.m.)

Andreas, it seems there is some problem with review board. I can see that you have posted
comments on the issues I raised. But the review board does not have a 'add comment' link
for the individual issues.

Any ways, for the range intersection problem, the start of one of the ranges should lie
in the other range. So the condition would be 
(a.start >= b.start && a.start <= b.end) || (b.start >= a.start && b.start <= a.end).
Correctness, and not the number of comparisons, is the first priority.

Andreas Hansson Oct. 31, 2012, 9:36 a.m. (Oct. 31, 2012, 9:36 a.m.)

What case would the current comparison get wrong? (just out of curiosity)

src/base/addr_range.hh (Diff revision 1)

Can we square brackets around the arguments, like the usual mathematical notation?

Andreas Hansson Oct. 31, 2012, 9:36 a.m. (Oct. 31, 2012, 9:36 a.m.)
```
Consider it done.
```

src/base/addr_range.hh (Diff revision 1)

Why this change in defintion of intersection? In fact, even the original definition was incorrect.

Show all issues

Description:

~		Changeset 9323:cfb472f99475
	~	Changeset 9325:dd82368c5370

		base: Encapsulate the underlying fields in AddrRange

		This patch makes the start and end address private in a move to
		prevent direct manipulation and matching of ranges based on these
		fields. This is done so that a transition to ranges with interleaving
		support is possible.

		As a result of hiding the start and end, a number of member functions
		are needed to perform the comparisons and manipulations that
		previously took place directly on the members. An accessor function is
		provided for the start address, and functions are added to merge two
		ranges, and test if an address is within a range. As a result of the
		latter the != and == operator is also removed in favour of the member
		function. A member function that returns a string representation is
		also created to allow debug printing.

		In general, this patch does not add any functionality, but it does
		take us closer to a situation where interleaving (and more cleverness)
		can be added under the bonnet without exposing it to the user. More on
		that in a later patch.

Diff:

Revision 2 (+133 -91)

Show changes

	src/arch/arm/linux/system.cc
	src/base/addr_range.hh
	src/base/addr_range_map.hh
	src/kern/tru64/tru64_events.cc
	src/mem/abstract_mem.hh
	src/mem/abstract_mem.cc
	src/mem/addr_mapper.cc
	src/mem/bus.hh
	2 more

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 9323:cfb472f99475

Changeset 9325:dd82368c5370

Diff:

Status: Closed (submitted)