From f55a4ce98960d4d624fb211a2b85567f0abe2bef Mon Sep 17 00:00:00 2001 From: Jason Lowe-Power Date: Thu, 17 Oct 2024 08:17:34 -0700 Subject: [PATCH] arch-x86,arch-arm: Remove static variables in decoders (#1643) There were a number of variables in the arm and x86 decoders that are static (e.g., the decode cache). It's a bit interesting that this doesn't cause problems with multiple cores since each core has its own decoder. However, this causes segfaults if you run different cores on different *host* threads. We are experimenting with running gem5 with multiple host thread (i.e., in parallel), and removing these static variables resolves the segfault. This change also adds const to any other static variables to ensure that they cannot be modified. Signed-off-by: Jason Lowe-Power --- src/arch/arm/decoder.cc | 2 -- src/arch/arm/decoder.hh | 2 +- src/arch/x86/decoder.cc | 5 ----- src/arch/x86/decoder.hh | 24 ++++++++++++------------ 4 files changed, 13 insertions(+), 20 deletions(-) diff --git a/src/arch/arm/decoder.cc b/src/arch/arm/decoder.cc index 9fc4be0e9a..3e898c5a47 100644 --- a/src/arch/arm/decoder.cc +++ b/src/arch/arm/decoder.cc @@ -53,8 +53,6 @@ namespace gem5 namespace ArmISA { -GenericISA::BasicDecodeCache Decoder::defaultCache; - Decoder::Decoder(const ArmDecoderParams ¶ms) : InstDecoder(params, &data), dvmEnabled(params.dvm_enabled), diff --git a/src/arch/arm/decoder.hh b/src/arch/arm/decoder.hh index 75488b6750..57c29546ae 100644 --- a/src/arch/arm/decoder.hh +++ b/src/arch/arm/decoder.hh @@ -94,7 +94,7 @@ class Decoder : public InstDecoder enums::DecoderFlavor decoderFlavor; /// A cache of decoded instruction objects. - static GenericISA::BasicDecodeCache defaultCache; + GenericISA::BasicDecodeCache defaultCache; friend class GenericISA::BasicDecodeCache; /** diff --git a/src/arch/x86/decoder.cc b/src/arch/x86/decoder.cc index af2456d6ab..ec595856a2 100644 --- a/src/arch/x86/decoder.cc +++ b/src/arch/x86/decoder.cc @@ -41,8 +41,6 @@ namespace gem5 namespace X86ISA { -X86ISAInst::MicrocodeRom Decoder::microcodeRom; - Decoder::State Decoder::doResetState() { @@ -671,9 +669,6 @@ Decoder::doImmediateState() return nextState; } -Decoder::InstBytes Decoder::dummy; -Decoder::InstCacheMap Decoder::instCacheMap; - StaticInstPtr Decoder::decode(ExtMachInst mach_inst, Addr addr) { diff --git a/src/arch/x86/decoder.hh b/src/arch/x86/decoder.hh index e4b1de96d7..eee48c1f76 100644 --- a/src/arch/x86/decoder.hh +++ b/src/arch/x86/decoder.hh @@ -60,19 +60,19 @@ class Decoder : public InstDecoder // These are defined and documented in decoder_tables.cc static const uint8_t SizeTypeToSize[3][10]; typedef const uint8_t ByteTable[256]; - static ByteTable Prefixes[2]; + static const ByteTable Prefixes[2]; - static ByteTable UsesModRMOneByte; - static ByteTable UsesModRMTwoByte; - static ByteTable UsesModRMThreeByte0F38; - static ByteTable UsesModRMThreeByte0F3A; + static const ByteTable UsesModRMOneByte; + static const ByteTable UsesModRMTwoByte; + static const ByteTable UsesModRMThreeByte0F38; + static const ByteTable UsesModRMThreeByte0F3A; - static ByteTable ImmediateTypeOneByte; - static ByteTable ImmediateTypeTwoByte; - static ByteTable ImmediateTypeThreeByte0F38; - static ByteTable ImmediateTypeThreeByte0F3A; + static const ByteTable ImmediateTypeOneByte; + static const ByteTable ImmediateTypeTwoByte; + static const ByteTable ImmediateTypeThreeByte0F38; + static const ByteTable ImmediateTypeThreeByte0F3A; - static X86ISAInst::MicrocodeRom microcodeRom; + X86ISAInst::MicrocodeRom microcodeRom; protected: using MachInst = uint64_t; @@ -88,7 +88,7 @@ class Decoder : public InstDecoder {} }; - static InstBytes dummy; + InstBytes dummy; // The bytes to be predecoded. MachInst fetchChunk; @@ -244,7 +244,7 @@ class Decoder : public InstDecoder decode_cache::InstMap *instMap = nullptr; typedef std::unordered_map< CacheKey, decode_cache::InstMap *> InstCacheMap; - static InstCacheMap instCacheMap; + InstCacheMap instCacheMap; StaticInstPtr decodeInst(ExtMachInst mach_inst);