Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/40106 )
Change subject: arch,cpu: Get rid of the RenameMode template class.
......................................................................
arch,cpu: Get rid of the RenameMode template class.
There is no way to make this sort of template work with more than one
ISA at a time, and it's also more complex than it needs to be,
particularly since the methods within it are never used in performance
critical code. Using virtual functions is also simpler and uses less
code.
Change-Id: I0baa1a651fa656420f6f90776572f8700a6d7cab
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40106
Reviewed-by: Giacomo Travaglini <[email protected]>
Maintainer: Giacomo Travaglini <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/arch/arm/isa.cc
M src/arch/arm/isa.hh
M src/arch/arm/utility.cc
M src/arch/generic/isa.hh
D src/arch/generic/traits.hh
M src/cpu/o3/cpu.cc
M src/cpu/o3/thread_context_impl.hh
7 files changed, 28 insertions(+), 113 deletions(-)
Approvals:
Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/isa.cc b/src/arch/arm/isa.cc
index 039224f..2f674cd 100644
--- a/src/arch/arm/isa.cc
+++ b/src/arch/arm/isa.cc
@@ -62,8 +62,7 @@
{
ISA::ISA(const Params &p) : BaseISA(p), system(NULL),
- _decoderFlavor(p.decoderFlavor), _vecRegRenameMode(Enums::Full),
- pmu(p.pmu), impdefAsNop(p.impdef_nop),
+ _decoderFlavor(p.decoderFlavor), pmu(p.pmu), impdefAsNop(p.impdef_nop),
afterStartup(false)
{
_regClasses.insert(_regClasses.end(), {
@@ -120,10 +119,6 @@
haveTME = true;
}
- // Initial rename mode depends on highestEL
- const_cast<Enums::VecRegRenameMode&>(_vecRegRenameMode) =
- highestELIs64 ? Enums::Full : Enums::Elem;
-
selfDebug = new SelfDebug();
initializeMiscRegMetadata();
preUnflattenMiscReg();
diff --git a/src/arch/arm/isa.hh b/src/arch/arm/isa.hh
index aa2a83a..c5c330f 100644
--- a/src/arch/arm/isa.hh
+++ b/src/arch/arm/isa.hh
@@ -50,7 +50,6 @@
#include "arch/arm/types.hh"
#include "arch/arm/utility.hh"
#include "arch/generic/isa.hh"
-#include "arch/generic/traits.hh"
#include "debug/Checkpoint.hh"
#include "enums/DecoderFlavor.hh"
#include "enums/VecRegRenameMode.hh"
@@ -71,7 +70,6 @@
// Micro Architecture
const Enums::DecoderFlavor _decoderFlavor;
- const Enums::VecRegRenameMode _vecRegRenameMode;
/** Dummy device for to handle non-existing ISA devices */
DummyISADevice dummyDevice;
@@ -878,9 +876,15 @@
}
Enums::VecRegRenameMode
- vecRegRenameMode() const
+ initVecRegRenameMode() const override
{
- return _vecRegRenameMode;
+ return highestELIs64 ? Enums::Full : Enums::Elem;
+ }
+
+ Enums::VecRegRenameMode
+ vecRegRenameMode(ThreadContext *_tc) const override
+ {
+ return _tc->pcState().aarch64() ? Enums::Full : Enums::Elem;
}
PARAMS(ArmISA);
@@ -902,32 +906,4 @@
};
}
-template<>
-struct RenameMode<ArmISA::ISA>
-{
- static Enums::VecRegRenameMode
- init(const BaseISA* isa)
- {
- auto arm_isa = dynamic_cast<const ArmISA::ISA *>(isa);
- assert(arm_isa);
- return arm_isa->vecRegRenameMode();
- }
-
- static Enums::VecRegRenameMode
- mode(const ArmISA::PCState& pc)
- {
- if (pc.aarch64()) {
- return Enums::Full;
- } else {
- return Enums::Elem;
- }
- }
-
- static bool
- equalsInit(const BaseISA* isa1, const BaseISA* isa2)
- {
- return init(isa1) == init(isa2);
- }
-};
-
#endif
diff --git a/src/arch/arm/utility.cc b/src/arch/arm/utility.cc
index 533b339..f5c792f 100644
--- a/src/arch/arm/utility.cc
+++ b/src/arch/arm/utility.cc
@@ -58,7 +58,7 @@
static void
copyVecRegs(ThreadContext *src, ThreadContext *dest)
{
- auto src_mode = RenameMode<ArmISA::ISA>::mode(src->pcState());
+ auto src_mode = src->getIsaPtr()->vecRegRenameMode(src);
// The way vector registers are copied (VecReg vs VecElem) is relevant
// in the O3 model only.
diff --git a/src/arch/generic/isa.hh b/src/arch/generic/isa.hh
index ae8fb39..ad5fa48 100644
--- a/src/arch/generic/isa.hh
+++ b/src/arch/generic/isa.hh
@@ -43,6 +43,7 @@
#include <vector>
#include "cpu/reg_class.hh"
+#include "enums/VecRegRenameMode.hh"
#include "sim/sim_object.hh"
class ThreadContext;
@@ -66,6 +67,18 @@
virtual uint64_t getExecutingAsid() const { return 0; }
virtual bool inUserMode() const = 0;
+ virtual Enums::VecRegRenameMode
+ initVecRegRenameMode() const
+ {
+ return Enums::Full;
+ }
+
+ virtual Enums::VecRegRenameMode
+ vecRegRenameMode(ThreadContext *_tc) const
+ {
+ return initVecRegRenameMode();
+ }
+
const RegClasses ®Classes() const { return _regClasses; }
};
diff --git a/src/arch/generic/traits.hh b/src/arch/generic/traits.hh
deleted file mode 100644
index 5363419..0000000
--- a/src/arch/generic/traits.hh
+++ /dev/null
@@ -1,68 +0,0 @@
-/*
- * Copyright (c) 2016, 2020 ARM Limited
- * All rights reserved
- *
- * The license below extends only to copyright in the software and shall
- * not be construed as granting a license to any other intellectual
- * property including but not limited to intellectual property relating
- * to a hardware implementation of the functionality of the software
- * licensed hereunder. You may use the software subject to the license
- * terms below provided that you ensure that this notice is replicated
- * unmodified and in its entirety in all distributions of the software,
- * modified or unmodified, in source code or in binary form.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met: redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer;
- * redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution;
- * neither the name of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-/* Auxiliary structs for architecture traits. */
-
-#ifndef __ARCH_COMMON_TRAITS_HH__
-#define __ARCH_COMMON_TRAITS_HH__
-
-#include "arch/generic/isa.hh"
-#include "arch/types.hh"
-#include "enums/VecRegRenameMode.hh"
-
-/** Helper structure to get the vector register mode for a given ISA.
- * This way we implement a default 'full' mode, and only those ISA that
care
- * have to actually specialise the template to forward the call to the
- * appropriate member of the ISA.
- */
-template <typename ISA>
-struct RenameMode
-{
- static Enums::VecRegRenameMode init(const BaseISA*) { return
Enums::Full; }
-
- static Enums::VecRegRenameMode
- mode(const TheISA::PCState&)
- { return Enums::Full; }
-
- /**
- * Compare the initial rename mode of two instances of the ISA.
- * Result is true by definition, as the default mode is Full.
- * */
- static bool equalsInit(const BaseISA*, const BaseISA*) { return true; }
-};
-
-#endif /* __ARCH_COMMON_TRAITS_HH__ */
diff --git a/src/cpu/o3/cpu.cc b/src/cpu/o3/cpu.cc
index 3cebbf1..04a9ee7 100644
--- a/src/cpu/o3/cpu.cc
+++ b/src/cpu/o3/cpu.cc
@@ -42,7 +42,6 @@
#include "cpu/o3/cpu.hh"
-#include "arch/generic/traits.hh"
#include "config/the_isa.hh"
#include "cpu/activity.hh"
#include "cpu/checker/cpu.hh"
@@ -89,7 +88,7 @@
/* It is mandatory that all SMT threads use the same renaming mode as
* they are sharing registers and rename */
- vecMode(RenameMode<TheISA::ISA>::init(params.isa[0])),
+ vecMode(params.isa[0]->initVecRegRenameMode()),
regFile(params.numPhysIntRegs,
params.numPhysFloatRegs,
params.numPhysVecRegs,
@@ -223,7 +222,8 @@
for (ThreadID tid = 0; tid < numThreads; tid++) {
isa[tid] = dynamic_cast<TheISA::ISA *>(params.isa[tid]);
assert(isa[tid]);
- assert(RenameMode<TheISA::ISA>::equalsInit(isa[tid], isa[0]));
+ assert(isa[tid]->initVecRegRenameMode() ==
+ isa[0]->initVecRegRenameMode());
// Only Alpha has an FP zero register, so for other ISAs we
// use an invalid FP register index to avoid special treatment
@@ -897,7 +897,7 @@
auto pc = this->pcState(tid);
// new_mode is the new vector renaming mode
- auto new_mode = RenameMode<TheISA::ISA>::mode(pc);
+ auto new_mode = isa[tid]->vecRegRenameMode(thread[tid]->getTC());
// We update vecMode only if there has been a change
if (new_mode != vecMode) {
diff --git a/src/cpu/o3/thread_context_impl.hh
b/src/cpu/o3/thread_context_impl.hh
index c132530..1a3a30d 100644
--- a/src/cpu/o3/thread_context_impl.hh
+++ b/src/cpu/o3/thread_context_impl.hh
@@ -42,7 +42,6 @@
#ifndef __CPU_O3_THREAD_CONTEXT_IMPL_HH__
#define __CPU_O3_THREAD_CONTEXT_IMPL_HH__
-#include "arch/generic/traits.hh"
#include "arch/registers.hh"
#include "config/the_isa.hh"
#include "cpu/o3/thread_context.hh"
@@ -151,7 +150,7 @@
O3ThreadContext<Impl>::copyArchRegs(ThreadContext *tc)
{
// Set vector renaming mode before copying registers
- cpu->vecRenameMode(RenameMode<TheISA::ISA>::mode(tc->pcState()));
+ cpu->vecRenameMode(tc->getIsaPtr()->vecRegRenameMode(tc));
// Prevent squashing
thread->noSquashFromTC = true;
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40106
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: I0baa1a651fa656420f6f90776572f8700a6d7cab
Gerrit-Change-Number: 40106
Gerrit-PatchSet: 12
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-Reviewer: Andreas Sandberg <[email protected]>
Gerrit-Reviewer: Daniel Carvalho <[email protected]>
Gerrit-Reviewer: Gabe Black <[email protected]>
Gerrit-Reviewer: Giacomo Travaglini <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
