Although they provide the exact same behavior, the params
created in the tests did not have the type expected by the
internal safe cast.
The following error was triggered:
storage.test.debug: build/NULL/base/cast.hh:47: T safe_cast(U)
[with T = const Stats::SampleStor::Params*;
U = const Stats::StorageParams*]:
Assertion `ret' failed.
Change-Id: I4f2ba51f3ccdb44589e61f235997245e7d9bf3c9
Signed-off-by: Daniel R. Carvalho <odanrc@yahoo.com.br>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40555
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
This "UnitTest" was really not a unit test, it was a timing utility for
measuring the performance of gem5's cprintf implementation. The name was
misleading, but more than that, it was linked against all of gem5 which
created a approximately 1.5 gigabyte binary for what is a very small
program.
Instead, the new version of cprintftime, which has the same
functionality as the old version, weighs in at a svelte 500k with debug
information.
This also trims down the number of misleading "UnitTest" entries to 3,
getting us closer to the point where we can eliminate that type of
entity entirely.
Change-Id: Id30d094f2844e948fe67e820c89412f8667aaa52
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40617
Maintainer: Gabe Black <gabe.black@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Create unit tests for the stats storage types. As a side effect
storage-related classes have been moved to separate files.
HistStor's grow_up, grow_out, and grow_convert have been made
private and renamed to comply with gem5's naming convention
and make grow_convert match its grow_up counterpart (growDown)
which is more suitable for its expected behavior.
The params declarations have been moved to be close to their
storage class' constructor.
HistStor has a explicit condition stating that there must be
at least 2 buckets.
Added documentation!
Fixed grow_convert so that it yields consistent histograms.
Previously buckets could not fully intersect, so doubling their
bucket size would make them steal contents innaproprietly. For
example, the neighbors [-6,-2[, [-2,2[, [2,6[, when doubled,
become [-12,-4[, [-4,4[, [4,12[; however, since the individual
values are not stored, it is impossible to know how to populate
the middle bucket with its neighbor's partial contents.
This fix forces the middle bucket of a storage to have its lower
bound at 0, solving the partial intersection issue.
Change-Id: Idb063e3dbda3cce3a8969e347660143162146eb9
Signed-off-by: Daniel R. Carvalho <odanrc@yahoo.com.br>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/25425
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Debug flags are flags that aid with debugging by printing
relevant information when enabled. Debug-formatting flags
define how the debug flags will print the information.
Although a viability, this patch does not support declaring
compound format flags.
As a side effect, now debug flags and debug-formatting flags
are printed in different lists, when using --debug-help.
Change-Id: Ieae68745276218cf4e9c1d37d7bf3bd1f19709ae
Signed-off-by: Daniel R. Carvalho <odanrc@yahoo.com.br>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39076
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
The base/refcnt.hh header was not used in base/types.hh at all, and
enum/ByteOrder.hh was there just so other files could find it. Instead,
this change moves enum/Byteorder.hh to sim/byteswap.hh where it's fits
with the purpose of the header.
This change also fixes some style problems with the code in
base/types.hh itself.
Change-Id: I471ae5cb2cca9169ba8616fb8411b40108a3ffb2
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39855
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
This was checking that the breakpoint length was equal to the length of
the ISA specific MachInst type. Instead, force the ISA specific remote
GDB subclass to implement a check if it wants to, specific to its needs.
The base implementation will just approve of any length, which should be
fine with a well behaved GDB client.
Change-Id: Id7325b788f8445049855f8104082b8e4da1fe300
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39661
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
This was mostly not used to begin with, but also when it was used, it
would obscure places where there were types, functions, etc, which were
switched between ISAs at compile time, and which would need to be
cleaned up to allow more than one ISA at a time.
Change-Id: Ieb372feff91b7e946b477fb78e54bcd0c2138966
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39655
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
As the std namespace expands, it becomes more and more likely that
blanketly importing all its symbols will cause a collision. Also, when
it was imported, the std:: was used or left off arbitrarily, sometimes
inconsistently even in the same function signature.
Change-Id: Ie30cbab154b00c60433908a206c229230d2b109f
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39536
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Gabe Black <gabe.black@gmail.com>
Maintainer: Gabe Black <gabe.black@gmail.com>
The warning happens when a key is present in the checkpoint but not in the
values that gem5 source code knows about.
To do this, we must expose iteration over IniFile section keys. To not
have to make those classes public, a visitor method is implemented.
Change-Id: I23340a953f3e604642b97690a7328b10fdd740a8
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/37575
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
The second argument in the std::max call is treated as an unsigned value
as all variables are unsigned as well. This will result in an
unsigned underflow, and as such the std::max is a no-op and will result
in the underflowed value.
The `start` and `used` value get corrupted after that, and checks for
`empty` and other stuff downstream break.
Change-Id: I00017e22ba84e65f6b1c596f47d348f342fbc304
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39496
Reviewed-by: Gabe Black <gabe.black@gmail.com>
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Comparing arrays with memcmp is fairly easy to do and will correctly
identify when a value is incorrect, but gtest doesn't know what
comparison actually happened and can't print any diagnostic information
to help the person running the test determine what went wrong.
Unfortunately, gtest is also slightly too smart and also not smart
enough to make it easy to compare the contents of sub-arrays with each
other. The thing you're checking the value of *must* be an array with a
well defined size (not a pointer), and the size *must* exactly match the
number of elements it expects to find.
One fairly clean way to get around this problem would be to use the new
std::span type introduced in c++20 which lets you refer to a sub-section
of another container in place, adjusting indexing, sizing, etc as
needed. Unfortunately since we only require up to c++-14 currently we
can't use that type.
Instead, we can create vectors which hold copies of the required data.
This is suboptimal since it means we're copying around data which
doesn't really need to be copied, but it means that the templates in
gtest will get a type they can handle, and the sizes will match like it
expects them to. Since the number of checks/copies is still small, the
overhead should be trivial in practice.
A helper function, subArr, has been added to help keep things fairly
clutter free.
Change-Id: I9f88c583a6a742346b177dba7cae791824b65942
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38895
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
The functions isSet(), noneSet(), and allSet() assume that
all bits of the underlying container have a corresponding
flag. This is typically not true, and the likelihood of
incorrectly using these functions is high.
Fortunately these functions are not being used anywhere,
and can be safely removed.
Alternatively, a mask could be provided on construction to
determine which bits of the underlying container correspond
to a flag bit, so that the proper bits are checked in these
functions.
Change-Id: Ia7cbfd0726943506a3f04dc417e67a0b57cdbf95
Signed-off-by: Daniel R. Carvalho <odanrc@yahoo.com.br>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38736
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
This prototype might convince the compiler that it should refer to
curTick indirectly through the linker, but curTick is inline (and making
it not has very high overhead), so there's a decent chance no non-inline
version will be emitted.
Change-Id: Iab5aacb145d4a974bc1bc0abdf7275c40fbb9c38
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38997
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Reviewed-by: Jason Lowe-Power <power.jg@gmail.com>
Maintainer: Jason Lowe-Power <power.jg@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
These functions return iterators which are inconsistent with the usage
model for this type. It should be accessed using the peek, push, and pop
methods and not iterators. If you need a class with iterators which is
oriented around accessing individual elements at a time, the
CircularQueue type is likely a better choice.
Change-Id: I9f37eab12e490b63d870d378a91f601dad353f25
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38998
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Maintainer: Gabe Black <gabe.black@gmail.com>
Maintainer: Andreas Sandberg <andreas.sandberg@arm.com>
Tested-by: kokoro <noreply+kokoro@google.com>
CircularQueue provides iterators which make it easier for users to
interact with it and helps abstract its internal state, but at the same
time it prevents standard algorithms like std::copy from recognizing
opportunities to use bulk copies to speed up execution. It also hides
the seams when wrapping around the buffer happens which std::copy
wouldn't know how to handle.
CircleBuf seems to be intended as a simpler type which doesn't hold
complex entries like the CircularQueue does, and instead just acts as a
wrap around buffer, like the name suggests. This change reimplements it
to not use CircularQueue, and to instead use an underlying vector.
The way internal indexing is handled CircularQueue was simplified
recently, and using the same scheme here means that this code is
actually not much more verbose than it was before. It also intrinsically
handles indexing and bulk accesses, and uses std::copy_n in a way that
lets it recognize and take advantage of contiguous storage and bulk
copies.
Change-Id: I78e7cfe174c52f60c95c81e5cd3d71c6052d4d41
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38896
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Gabe Black <gabe.black@gmail.com>
Maintainer: Andreas Sandberg <andreas.sandberg@arm.com>
Tested-by: kokoro <noreply+kokoro@google.com>
This method lets you stretch the current chunk, if you want to skip over
some of the upcoming chunks since you've already handled their bytes.
For instance, if you were iterating over pages in a range of virtual
addresses, you might be able to handle multiple page sized chunks at
once if they were represented by a single large sized page table entry.
This mechanism would let you move past all the pages you had just
handled without having to walk through them all one by one.
Change-Id: I7d962f548947b77f0aa1b725036dbcc9e1b28659
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38718
Reviewed-by: Gabe Black <gabe.black@gmail.com>
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
This class had been trying to keep all indices within the modulus of the
queue size, and to use all elements in the underlying storage by making
the empty and full conditions alias, differentiated by a bool. To keep
track of the difference between a storage location on one trip around
the queue vs other times around, ie an alias once the indices had
wrapped, it also keep track of a "round" value in both the queue itself,
and any iterators it created.
All this bookkeeping significantly complicated the data structure.
Instead, this change modifies it to keep track of a monotonically
increasing index which is wrapped at the time it's used. Only the head
index and current size need to be tracked in the queue itself, and only
a pointer to the queue and an index need to be tracked in the iterators.
Theoretically, it's possible that this value could overflow eventually
since it increases forever, unlike before where the index wrapped and
was never larger than the queue's capacity. In practice, the type of the
index was changed from a uint32_t to a size_t, probably a 64 bit value
in modern systems, which will hold much larger values. Also, the round
counter and the index values together acted like a smaller than 64 bit
value anyway, since the round counter would overflow after 2^32 times
around a less than 2^32 entry queue.
One minor interface difference is that the head() and tail() values
returned by the queue are no longer pre-wrapped to be modulo the queue's
capacity. As long as consumers don't try to be overly clever and feed in
fixed values, do their own bounds checking, etc., something that would
be cumbersome considering the wrapping nature of the structure, this
shouldn't be an issue.
Also, since external consumers no longer need to worry about wrapping,
since only one of them was used in only one place, and because they
weren't even marked as part of the interface, the modulo helper
functions have been eliminated from the queue. If other code wants to
perform modulo arithmetic for some reason (which the queue no longer
requires) they can accomplish basically the same thing in basically the
same amount of code using normal math.
Also, rather than inherit from std::vector, this change makes the vector
internal to the queue. That prevents methods of the vector that aren't
aware of the circular nature of the structure from leaking out if
they're not overridden or otherwise proactively blocked.
On top of simplifying the implementation, this also makes it perform
*slightly* better. To measure that, I ran the following command:
$ time build/ARM/base/circular_queue.test.opt --gtest_repeat=100000 > /dev/null
and found a few percent improvement in total run time. While this
difference was small and not measured against realistic usage of the
data structure, it was still measurable, and at minimum doesn't seem to
have hurt performance.
Change-Id: Ic2baa28de135be7086fa94579bbec451d69b3b15
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38478
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
If an `AddrRange` is not interleaved, return the input address in
`removeIntlvBits` and `addIntlvBits` to prevent undefined behavior. It
allows to use these methods in all cases without having to check
manually whether the range is interleaved.
Change-Id: Ic6ac8c4e52b09417bc41aa9380a24319c34e0b35
Signed-off-by: Isaac Sánchez Barrera <isaac.sanchez@bsc.es>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/37617
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
The methods `AddrRange::removeIntlvBits(Addr)` and
`AddrRange::addIntlvBits(Addr)` should be the inverse of one another,
but the latter did not insert the blanks for filling the removed bits in
the correct positions. Since the masks are ordered increasingly by the
position of the least significant bit of each mask, the lowest bit that
has to be inserted at each iteration is always `intlv_bit`, not needing
to be shifted to the left or right. The bits that need to be copied
from the input address are `intlv_bit-1..0` at each iteration.
The test `AddrRangeTest.AddRemoveInterleavBitsAcrossRange` has been
updated have masks below bit 12, making the old code not pass the test.
A new `AddrRangeTest.AddRemoveInterleavBitsAcrossContiguousRange` test
has been added to include a case in which the previous code fails. The
corrected code passes both tests.
This function is not used anywhere other than the tests and the class
`ChannelAddr`. However, it is needed to efficiently implement
interleaved caches in the classic mode.
Change-Id: I7d626a1f6ecf09a230fc18810d2dad2104d1a865
Signed-off-by: Isaac Sánchez Barrera <isaac.sanchez@bsc.es>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/37175
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Reviewed-by: Bobby R. Bruce <bbruce@ucdavis.edu>
Maintainer: Nikos Nikoleris <nikos.nikoleris@arm.com>
Maintainer: Bobby R. Bruce <bbruce@ucdavis.edu>
