From 48ac1ea38dce6d7068ba4c0f95a3046812ed787c Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Fri, 28 Jul 2023 01:01:50 -0700 Subject: [PATCH 1/2] gpu-compute: "" -> "base/random.hh" in testers/gpu... In "src/cpu/testers/gpu_ruby_test" a random number generator was used. This was using the CPP "" library. This patch changes it to the gem5 random class (that declared in "base/random.hh"). In addition to this, undeterministic behavior has been removed. Via "protocol_tester.cc" the RNG is either seeded with a seed specified by the user, or goes with the gem5 default seed. This ensures reproducable runs. Prior to this patch the RNG was seeded with `time(NULL)`. This made finding faults difficult. Change-Id: Ia8e9f7b87e91323f828e0b7f6c3906c0c5793b2c --- src/cpu/testers/gpu_ruby_test/ProtocolTester.py | 2 +- src/cpu/testers/gpu_ruby_test/address_manager.cc | 12 ++++++++---- src/cpu/testers/gpu_ruby_test/episode.cc | 3 ++- src/cpu/testers/gpu_ruby_test/protocol_tester.cc | 10 +++++----- src/cpu/testers/gpu_ruby_test/tester_thread.cc | 4 +++- 5 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/cpu/testers/gpu_ruby_test/ProtocolTester.py b/src/cpu/testers/gpu_ruby_test/ProtocolTester.py index 178376bee9..c314392520 100644 --- a/src/cpu/testers/gpu_ruby_test/ProtocolTester.py +++ b/src/cpu/testers/gpu_ruby_test/ProtocolTester.py @@ -73,7 +73,7 @@ class ProtocolTester(ClockedObject): random_seed = Param.Int( 0, "Random seed number. Default value (0) means \ - using runtime-specific value.", + using base/random.hh without seed.", ) log_file = Param.String("Log file's name") system = Param.System(Parent.any, "System we belong to") diff --git a/src/cpu/testers/gpu_ruby_test/address_manager.cc b/src/cpu/testers/gpu_ruby_test/address_manager.cc index 049ba86e51..a0c0670a8f 100644 --- a/src/cpu/testers/gpu_ruby_test/address_manager.cc +++ b/src/cpu/testers/gpu_ruby_test/address_manager.cc @@ -33,7 +33,6 @@ #include #include -#include #include "base/intmath.hh" #include "base/logging.hh" @@ -101,7 +100,8 @@ AddressManager::getAddress(Location loc) AddressManager::Location AddressManager::getAtomicLoc() { - Location ret_atomic_loc = random() % numAtomicLocs; + Location ret_atomic_loc = \ + random_mt.random() % numAtomicLocs; atomicStructs[ret_atomic_loc]->startLocSelection(); return ret_atomic_loc; } @@ -206,7 +206,9 @@ AddressManager::AtomicStruct::getLoadLoc() // we can pick any location btw // locArray [firstMark : arraySize-1] int range_size = arraySize - firstMark; - Location ret_loc = locArray[firstMark + random() % range_size]; + Location ret_loc = locArray[ + firstMark + random_mt.random() % range_size + ]; // update loadStoreMap LdStMap::iterator it = loadStoreMap.find(ret_loc); @@ -238,7 +240,9 @@ AddressManager::AtomicStruct::getStoreLoc() } else { // we can pick any location btw [firstMark : secondMark-1] int range_size = secondMark - firstMark; - Location ret_loc = locArray[firstMark + random() % range_size]; + Location ret_loc = locArray[ + firstMark + random_mt.random() % range_size + ]; // update loadStoreMap LdStMap::iterator it = loadStoreMap.find(ret_loc); diff --git a/src/cpu/testers/gpu_ruby_test/episode.cc b/src/cpu/testers/gpu_ruby_test/episode.cc index 6822049bbd..7e16b0ef07 100644 --- a/src/cpu/testers/gpu_ruby_test/episode.cc +++ b/src/cpu/testers/gpu_ruby_test/episode.cc @@ -34,6 +34,7 @@ #include #include +#include "base/random.hh" #include "cpu/testers/gpu_ruby_test/protocol_tester.hh" #include "cpu/testers/gpu_ruby_test/tester_thread.hh" @@ -100,7 +101,7 @@ Episode::initActions() int num_loads = numLoads; int num_stores = numStores; while ((num_loads + num_stores) > 0) { - switch (random() % 2) { + switch (random_mt.random() % 2) { case 0: // Load if (num_loads > 0) { actions.push_back(new Action(Action::Type::LOAD, diff --git a/src/cpu/testers/gpu_ruby_test/protocol_tester.cc b/src/cpu/testers/gpu_ruby_test/protocol_tester.cc index f2fd7f9600..fdf30304f1 100644 --- a/src/cpu/testers/gpu_ruby_test/protocol_tester.cc +++ b/src/cpu/testers/gpu_ruby_test/protocol_tester.cc @@ -34,8 +34,8 @@ #include #include #include -#include +#include "base/random.hh" #include "cpu/testers/gpu_ruby_test/cpu_thread.hh" #include "cpu/testers/gpu_ruby_test/dma_thread.hh" #include "cpu/testers/gpu_ruby_test/gpu_wavefront.hh" @@ -141,11 +141,11 @@ ProtocolTester::ProtocolTester(const Params &p) sentExitSignal = false; - // set random seed number + // set random seed number, if specified. + // Note: random_m5 will use a fixed key if random_seed is not set. + // This ensures a reproducable. if (p.random_seed != 0) { - srand(p.random_seed); - } else { - srand(time(NULL)); + random_mt.init(p.random_seed); } actionCount = 0; diff --git a/src/cpu/testers/gpu_ruby_test/tester_thread.cc b/src/cpu/testers/gpu_ruby_test/tester_thread.cc index 760f8c2d87..ce3a1bccc6 100644 --- a/src/cpu/testers/gpu_ruby_test/tester_thread.cc +++ b/src/cpu/testers/gpu_ruby_test/tester_thread.cc @@ -33,6 +33,7 @@ #include +#include "base/random.hh" #include "debug/ProtocolTest.hh" namespace gem5 @@ -144,7 +145,8 @@ TesterThread::attachTesterThreadToPorts(ProtocolTester *_tester, void TesterThread::issueNewEpisode() { - int num_reg_loads = random() % tester->getEpisodeLength(); + int num_reg_loads = \ + random_mt.random() % tester->getEpisodeLength(); int num_reg_stores = tester->getEpisodeLength() - num_reg_loads; // create a new episode From 08a3762a14d9905f3eda19d96a10e69ea7e45121 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Fri, 28 Jul 2023 10:02:04 -0700 Subject: [PATCH 2/2] gpu-compute: Add warn for `random_seed == 0` case Addresses: https://github.com/gem5/gem5/pull/140#pullrequestreview-1552383650 Change-Id: Ia09a2bc74f35d3d6cb066efaf9d113db6caf4557 --- src/cpu/testers/gpu_ruby_test/protocol_tester.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/cpu/testers/gpu_ruby_test/protocol_tester.cc b/src/cpu/testers/gpu_ruby_test/protocol_tester.cc index fdf30304f1..6b3f9e19f1 100644 --- a/src/cpu/testers/gpu_ruby_test/protocol_tester.cc +++ b/src/cpu/testers/gpu_ruby_test/protocol_tester.cc @@ -146,6 +146,15 @@ ProtocolTester::ProtocolTester(const Params &p) // This ensures a reproducable. if (p.random_seed != 0) { random_mt.init(p.random_seed); + } else { + warn( + "If `random_seed == 0` (or `random_seed` is unset) " + "ProtocolTester does not seed the RNG. This will NOT result in " + "the RNG generating different results each run. In this case the " + "RNG is seeded by a default value. This differs from behavior in " + "previous versions of gem5. Setting `random_seed` to a non-zero " + "value is strongly recommended." + ); } actionCount = 0;