[gem5-dev] Change in gem5/gem5[develop]: cpu: Remove automatic overriding of numThreads in SE on O3.

2020-10-14 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/35937 )


Change subject: cpu: Remove automatic overriding of numThreads in SE on O3.
..

cpu: Remove automatic overriding of numThreads in SE on O3.

On the O3 CPU, when the number of threads on the CPU (SMT) is too low to
hold all the old style CPU workload items, then it would increase the
number of threads to match. There are three problems with this.

1. This behavior was only implemented on O3.
2. It could silently hide a bug in the config where the number of
   workload items was accidentally too big.
3. It makes the DerivO3CPUParams struct tamper with itself in the
   create() method, which means not even config.ini will accurately
   reflect the actual config of the system.

Change-Id: I0aab70d4b98093f7f14156ca437e763f031049ab
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/35937
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M src/cpu/o3/cpu.cc
M src/cpu/o3/deriv.cc
2 files changed, 4 insertions(+), 14 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/cpu/o3/cpu.cc b/src/cpu/o3/cpu.cc
index 20274e0..c5043a7 100644
--- a/src/cpu/o3/cpu.cc
+++ b/src/cpu/o3/cpu.cc
@@ -132,6 +132,10 @@
 fatal_if(FullSystem && params->numThreads > 1,
 "SMT is not supported in O3 in full system mode currently.");

+fatal_if(!FullSystem && params->numThreads < params->workload.size(),
+"More workload items (%d) than threads (%d) on CPU %s.",
+params->workload.size(), params->numThreads, name());
+
 if (!params->switched_out) {
 _status = Running;
 } else {
diff --git a/src/cpu/o3/deriv.cc b/src/cpu/o3/deriv.cc
index 68baae4..5da710f 100644
--- a/src/cpu/o3/deriv.cc
+++ b/src/cpu/o3/deriv.cc
@@ -35,19 +35,5 @@
 DerivO3CPU *
 DerivO3CPUParams::create()
 {
-if (!FullSystem) {
-if (workload.size() > numThreads) {
-fatal("Workload Size (%i) > Max Supported Threads (%i) on This  
CPU",

-  workload.size(), numThreads);
-} else if (workload.size() == 0) {
-fatal("Must specify at least one workload!");
-}
-
-// In non-full-system mode, we infer the number of threads from
-// the workload if it's not explicitly specified.
-numThreads =
-(numThreads >= workload.size()) ? numThreads : workload.size();
-}
-
 return new DerivO3CPU(this);
 }

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/35937
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I0aab70d4b98093f7f14156ca437e763f031049ab
Gerrit-Change-Number: 35937
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Nikos Nikoleris 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: cpu: Remove automatic overriding of numThreads in SE on O3.

2020-10-13 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/35937 )



Change subject: cpu: Remove automatic overriding of numThreads in SE on O3.
..

cpu: Remove automatic overriding of numThreads in SE on O3.

On the O3 CPU, when the number of threads on the CPU (SMT) is too low to
hold all the old style CPU workload items, then it would increase the
number of threads to match. There are three problems with this.

1. This behavior was only implemented on O3.
2. It could silently hide a bug in the config where the number of
   workload items was accidentally too big.
3. It makes the DerivO3CPUParams struct tamper with itself in the
   create() method, which means not even config.ini will accurately
   reflect the actual config of the system.

Change-Id: I0aab70d4b98093f7f14156ca437e763f031049ab
---
M src/cpu/o3/cpu.cc
M src/cpu/o3/deriv.cc
2 files changed, 4 insertions(+), 14 deletions(-)



diff --git a/src/cpu/o3/cpu.cc b/src/cpu/o3/cpu.cc
index 20274e0..c5043a7 100644
--- a/src/cpu/o3/cpu.cc
+++ b/src/cpu/o3/cpu.cc
@@ -132,6 +132,10 @@
 fatal_if(FullSystem && params->numThreads > 1,
 "SMT is not supported in O3 in full system mode currently.");

+fatal_if(!FullSystem && params->numThreads < params->workload.size(),
+"More workload items (%d) than threads (%d) on CPU %s.",
+params->workload.size(), params->numThreads, name());
+
 if (!params->switched_out) {
 _status = Running;
 } else {
diff --git a/src/cpu/o3/deriv.cc b/src/cpu/o3/deriv.cc
index 461725f..5da710f 100644
--- a/src/cpu/o3/deriv.cc
+++ b/src/cpu/o3/deriv.cc
@@ -35,19 +35,5 @@
 DerivO3CPU *
 DerivO3CPUParams::create()
 {
-if (!FullSystem) {
-if (workload.size() > numThreads) {
-fatal("Workload Size (%i) > Max Supported Threads (%i) on This  
CPU",

-  workload.size(), numThreads);
-} else if (workload.size() == 0) {
-fatal("Must specify at least one workload!");
-}
-
-// In non-full-system mode, we infer the number of threads from
-// the workload if it's not explicitly specified.
-num_threads =
-(numThreads >= workload.size()) ? numThreads : workload.size();
-}
-
 return new DerivO3CPU(this);
 }

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/35937
To unsubscribe, or for help writing mail filters, visit  
https://gem5-review.googlesource.com/settings


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I0aab70d4b98093f7f14156ca437e763f031049ab
Gerrit-Change-Number: 35937
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s