[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Create a separate type for floating point reg idxs.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42354 )



Change subject: arch-x86: Create a separate type for floating point reg  
idxs.

..

arch-x86: Create a separate type for floating point reg idxs.

This will ensure that floating point registers are used when required
and never otherwise.

Change-Id: I303c42d8a74c56b7b433b91fd36dc6aaf5ddef32
---
M src/arch/x86/insts/microldstop.hh
M src/arch/x86/insts/microop_args.hh
M src/arch/x86/insts/static_inst.hh
M src/arch/x86/isa/includes.isa
M  
src/arch/x86/isa/insts/simd128/integer/save_and_restore_state/save_and_restore_state.py

M src/arch/x86/isa/microasm.isa
6 files changed, 29 insertions(+), 18 deletions(-)



diff --git a/src/arch/x86/insts/microldstop.hh  
b/src/arch/x86/insts/microldstop.hh

index 26178cb..228c68d 100644
--- a/src/arch/x86/insts/microldstop.hh
+++ b/src/arch/x86/insts/microldstop.hh
@@ -101,7 +101,7 @@
 {
   protected:
 LdStFpOp(ExtMachInst mach_inst, const char *mnem, const char  
*inst_mnem,

-uint64_t set_flags, InstRegIndex _data,
+uint64_t set_flags, FpRegIndex _data,
 uint8_t _scale, InstRegIndex _index, InstRegIndex _base,
 uint64_t _disp, InstRegIndex _segment,
 uint8_t data_size, uint8_t address_size,
diff --git a/src/arch/x86/insts/microop_args.hh  
b/src/arch/x86/insts/microop_args.hh

index 85f60d1..2025e04 100644
--- a/src/arch/x86/insts/microop_args.hh
+++ b/src/arch/x86/insts/microop_args.hh
@@ -194,10 +194,10 @@
 template 
 struct FloatOp : public Base
 {
+using ArgType = FpRegIndex;
+
 template 
-FloatOp(InstType *inst, typename Base::ArgType idx) :
-Base(idx.index, inst->dataSize)
-{}
+FloatOp(InstType *inst, ArgType idx) : Base(idx.index, inst->dataSize)  
{}


 void
 print(std::ostream ) const
diff --git a/src/arch/x86/insts/static_inst.hh  
b/src/arch/x86/insts/static_inst.hh

index d2768b6..9d5039b 100644
--- a/src/arch/x86/insts/static_inst.hh
+++ b/src/arch/x86/insts/static_inst.hh
@@ -46,7 +46,7 @@
 {

 /**
- * Class for register indices passed to instruction constructors. Using a
+ * Classes for register indices passed to instruction constructors. Using a
  * wrapper struct for these lets take advantage of the compiler's type
  * checking.
  */
@@ -56,6 +56,12 @@
 explicit InstRegIndex(RegIndex idx) : index(idx) {}
 };

+struct FpRegIndex
+{
+RegIndex index;
+explicit FpRegIndex(RegIndex idx) : index(idx) {}
+};
+
 /**
  * Base class for all X86 static instructions.
  */
diff --git a/src/arch/x86/isa/includes.isa b/src/arch/x86/isa/includes.isa
index 070d82f..59a4f9e 100644
--- a/src/arch/x86/isa/includes.isa
+++ b/src/arch/x86/isa/includes.isa
@@ -72,6 +72,8 @@
 #include "sim/faults.hh"

 using X86ISA::InstRegIndex;
+using X86ISA::FpRegIndex;
+
 }};

 output decoder {{
diff --git  
a/src/arch/x86/isa/insts/simd128/integer/save_and_restore_state/save_and_restore_state.py  
b/src/arch/x86/isa/insts/simd128/integer/save_and_restore_state/save_and_restore_state.py

index c519b56..36a3952 100644
---  
a/src/arch/x86/isa/insts/simd128/integer/save_and_restore_state/save_and_restore_state.py
+++  
b/src/arch/x86/isa/insts/simd128/integer/save_and_restore_state/save_and_restore_state.py

@@ -43,16 +43,16 @@
 '''

 loadXMMRegTemplate =  '''
-ldfp regIdx("FLOATREG_XMM_LOW(%(idx)i)"), seg, %(mode)s, \
+ldfp fpRegIdx("FLOATREG_XMM_LOW(%(idx)i)"), seg, %(mode)s, \
  "DISPLACEMENT + 160 + 16 * %(idx)i", dataSize=8
-ldfp regIdx("FLOATREG_XMM_HIGH(%(idx)i)"), seg, %(mode)s, \
+ldfp fpRegIdx("FLOATREG_XMM_HIGH(%(idx)i)"), seg, %(mode)s, \
  "DISPLACEMENT + 160 + 16 * %(idx)i + 8", dataSize=8
 '''

 storeXMMRegTemplate =  '''
-stfp regIdx("FLOATREG_XMM_LOW(%(idx)i)"), seg, %(mode)s, \
+stfp fpRegIdx("FLOATREG_XMM_LOW(%(idx)i)"), seg, %(mode)s, \
  "DISPLACEMENT + 160 + 16 * %(idx)i", dataSize=8
-stfp regIdx("FLOATREG_XMM_HIGH(%(idx)i)"), seg, %(mode)s, \
+stfp fpRegIdx("FLOATREG_XMM_HIGH(%(idx)i)"), seg, %(mode)s, \
  "DISPLACEMENT + 160 + 16 * %(idx)i + 8", dataSize=8
 '''

diff --git a/src/arch/x86/isa/microasm.isa b/src/arch/x86/isa/microasm.isa
index 59551a5..3f0517d 100644
--- a/src/arch/x86/isa/microasm.isa
+++ b/src/arch/x86/isa/microasm.isa
@@ -57,16 +57,19 @@
 assembler = MicroAssembler(X86Macroop, microopClasses, mainRom,  
Rom_Macroop)


 def regIdx(idx):
-return "InstRegIndex(%s)" % idx
+return "X86ISA::InstRegIndex(%s)" % idx
+def fpRegIdx(idx):
+return "X86ISA::FpRegIndex(%s)" % idx

 assembler.symbols["regIdx"] = regIdx
+assembler.symbols["fpRegIdx"] = fpRegIdx

 # Add in symbols for the microcode registers
 for num in range(16):
 assembler.symbols["t%d" % num] = regIdx("NUM_INTREGS+%d" % num)
 for num in range(8):
 

[gem5-dev] Change in gem5/gem5[develop]: base: Make the functions in intmath.hh constexpr.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42357 )



Change subject: base: Make the functions in intmath.hh constexpr.
..

base: Make the functions in intmath.hh constexpr.

These simple functions can potentially be evaluated at compile time, and
marking them constexpr makes them available in more contexts.

Change-Id: I9cf39c517e7c53c276883f311739c1b153ccfd44
---
M src/base/intmath.hh
1 file changed, 7 insertions(+), 7 deletions(-)



diff --git a/src/base/intmath.hh b/src/base/intmath.hh
index 473f0e3..acf7681 100644
--- a/src/base/intmath.hh
+++ b/src/base/intmath.hh
@@ -39,7 +39,7 @@
 /**
  * @ingroup api_base_utils
  */
-inline uint64_t
+static constexpr uint64_t
 power(uint32_t n, uint32_t e)
 {
 uint64_t result = 1;
@@ -59,7 +59,7 @@
  * @ingroup api_base_utils
  */
 template 
-inline typename std::enable_if_t::value, int>
+static constexpr std::enable_if_t::value, int>
 floorLog2(T x)
 {
 assert(x > 0);
@@ -84,7 +84,7 @@
  * @ingroup api_base_utils
  */
 template 
-inline int
+static constexpr int
 ceilLog2(const T& n)
 {
 assert(n > 0);
@@ -98,7 +98,7 @@
  * @ingroup api_base_utils
  */
 template 
-inline bool
+static constexpr bool
 isPowerOf2(const T& n)
 {
 // If n is non-zero, and subtracting one borrows all the way to the MSB
@@ -110,7 +110,7 @@
  * @ingroup api_base_utils
  */
 template 
-inline T
+static constexpr T
 divCeil(const T& a, const U& b)
 {
 return (a + b - 1) / b;
@@ -127,7 +127,7 @@
  * @ingroup api_base_utils
  */
 template 
-inline T
+static constexpr T
 roundUp(const T& val, const U& align)
 {
 assert(isPowerOf2(align));
@@ -146,7 +146,7 @@
  * @ingroup api_base_utils
  */
 template 
-inline T
+static constexpr T
 roundDown(const T& val, const U& align)
 {
 assert(isPowerOf2(align));

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42357
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: I9cf39c517e7c53c276883f311739c1b153ccfd44
Gerrit-Change-Number: 42357
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

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Fix style and use uop args in seqop.isa.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42351 )



Change subject: arch-x86: Fix style and use uop args in seqop.isa.
..

arch-x86: Fix style and use uop args in seqop.isa.

Change-Id: I41ed7f0aa8dd00ed0f6f8361837945810d12bf9e
---
M src/arch/x86/insts/microop_args.hh
M src/arch/x86/isa/microops/seqop.isa
2 files changed, 100 insertions(+), 137 deletions(-)



diff --git a/src/arch/x86/insts/microop_args.hh  
b/src/arch/x86/insts/microop_args.hh

index ac4d81e..6fa09f5 100644
--- a/src/arch/x86/insts/microop_args.hh
+++ b/src/arch/x86/insts/microop_args.hh
@@ -264,6 +264,22 @@
 }
 };

+struct UpcOp
+{
+using ArgType = MicroPC;
+
+MicroPC target;
+
+template 
+UpcOp(InstType *inst, ArgType _target) : target(_target) {}
+
+void
+print(std::ostream ) const
+{
+ccprintf(os, "%#x", target);
+}
+};
+
 struct FaultOp
 {
 using ArgType = Fault;
diff --git a/src/arch/x86/isa/microops/seqop.isa  
b/src/arch/x86/isa/microops/seqop.isa

index 64f749b..7b52101 100644
--- a/src/arch/x86/isa/microops/seqop.isa
+++ b/src/arch/x86/isa/microops/seqop.isa
@@ -33,114 +33,81 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

-output header {{
-class SeqOpBase : public X86ISA::X86MicroopBase
-{
-  protected:
-uint16_t target;
-uint8_t cc;
-
-  public:
-SeqOpBase(ExtMachInst _machInst, const char * instMnem,
-const char * mnemonic, uint64_t setFlags,
-uint16_t _target, uint8_t _cc);
-
-SeqOpBase(ExtMachInst _machInst, const char * instMnem,
-const char * mnemonic,
-uint16_t _target, uint8_t _cc);
-
-std::string generateDisassembly(Addr pc,
-const Loader::SymbolTable *symtab) const override;
-};
-}};
-
-def template SeqOpDeclare {{
+def template BrDeclare {{
 class %(class_name)s : public %(base_class)s
 {
   private:
 %(reg_idx_arr_decl)s;

   public:
-%(class_name)s(ExtMachInst _machInst, const char * instMnem,
-uint64_t setFlags, uint16_t _target, uint8_t _cc);
+%(class_name)s(ExtMachInst mach_inst, const char *inst_mnem,
+uint64_t set_flags, uint16_t _target, uint8_t _cc) :
+%(base_class)s(mach_inst, "%(mnemonic)s", inst_mnem, set_flags,
+%(op_class)s, _target, _cc)
+{
+%(set_reg_idx_arr)s;
+%(constructor)s;
+}

 Fault execute(ExecContext *, Trace::InstRecord *) const override;

-X86ISA::PCState branchTarget(const X86ISA::PCState ) const
-override;
+X86ISA::PCState
+branchTarget(const X86ISA::PCState ) const override
+{
+X86ISA::PCState pcs = branchPC;
+DPRINTF(X86, "Br branchTarget PC info: %s, Target: %d\n",
+pcs, (int16_t)target);
+pcs.nupc(target);
+pcs.uAdvance();
+return pcs;
+}

 /// Explicitly import the otherwise hidden branchTarget
 using StaticInst::branchTarget;
 };
 }};

-def template SeqOpExecute {{
-Fault %(class_name)s::execute(ExecContext *xc,
-Trace::InstRecord *traceData) const
+def template EretDeclare {{
+class %(class_name)s : public %(base_class)s
+{
+  private:
+%(reg_idx_arr_decl)s;
+
+  public:
+%(class_name)s(ExtMachInst mach_inst, const char *inst_mnem,
+uint64_t set_flags, uint8_t _cc) :
+%(base_class)s(mach_inst, "%(mnemonic)s", inst_mnem, set_flags,
+%(op_class)s, _cc)
 {
-%(op_decl)s;
-%(op_rd)s;
-if (%(cond_test)s) {
-%(code)s;
-} else {
-%(else_code)s;
-}
-%(op_wb)s;
-return NoFault;
+%(set_reg_idx_arr)s;
+%(constructor)s;
 }
+
+Fault execute(ExecContext *, Trace::InstRecord *) const override;
+};
 }};

-output decoder {{
-SeqOpBase::SeqOpBase(
-ExtMachInst machInst, const char * mnemonic, const char *  
instMnem,

-uint64_t setFlags, uint16_t _target, uint8_t _cc) :
-X86MicroopBase(machInst, mnemonic, instMnem, setFlags, No_OpClass),
-target(_target), cc(_cc)
+def template SeqOpExecute {{
+Fault
+%(class_name)s::execute(ExecContext *xc,
+Trace::InstRecord *traceData) const
 {
-}
-}};
-
-def template SeqOpConstructor {{
-%(class_name)s::%(class_name)s(
-ExtMachInst machInst, const char * instMnem,
-uint64_t setFlags, uint16_t _target, uint8_t _cc) :
-%(base_class)s(machInst, "%(mnemonic)s", instMnem,
-

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Correct style and use uop args in specop.isa.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42350 )



Change subject: arch-x86: Correct style and use uop args in specop.isa.
..

arch-x86: Correct style and use uop args in specop.isa.

Also spin fixed code out into header files.

Change-Id: I1b326c8cb999d797102ba36b5c13850023a50615
---
M src/arch/x86/insts/microop.hh
M src/arch/x86/insts/microop_args.hh
A src/arch/x86/insts/microspecop.hh
M src/arch/x86/isa/includes.isa
M src/arch/x86/isa/microops/specop.isa
5 files changed, 120 insertions(+), 105 deletions(-)



diff --git a/src/arch/x86/insts/microop.hh b/src/arch/x86/insts/microop.hh
index ac12cec..13c4bee 100644
--- a/src/arch/x86/insts/microop.hh
+++ b/src/arch/x86/insts/microop.hh
@@ -141,6 +141,20 @@
 using StaticInst::branchTarget;
 };

+class MicroCondBase : public X86MicroopBase
+{
+  protected:
+uint8_t cc;
+
+  public:
+MicroCondBase(ExtMachInst mach_inst, const char *mnem,
+const char *inst_mnem, uint64_t set_flags, OpClass op_class,
+uint8_t _cc) :
+X86MicroopBase(mach_inst, mnem, inst_mnem, set_flags, op_class),
+cc(_cc)
+{}
+};
+
 }

 #endif //__ARCH_X86_INSTS_MICROOP_HH__
diff --git a/src/arch/x86/insts/microop_args.hh  
b/src/arch/x86/insts/microop_args.hh

index a3ebe29..ac4d81e 100644
--- a/src/arch/x86/insts/microop_args.hh
+++ b/src/arch/x86/insts/microop_args.hh
@@ -37,6 +37,7 @@
 #include "arch/x86/types.hh"
 #include "base/cprintf.hh"
 #include "cpu/reg_class.hh"
+#include "sim/faults.hh"

 namespace X86ISA
 {
@@ -263,6 +264,22 @@
 }
 };

+struct FaultOp
+{
+using ArgType = Fault;
+
+Fault fault;
+
+template 
+FaultOp(InstType *inst, ArgType _fault) : fault(_fault) {}
+
+void
+print(std::ostream ) const
+{
+ccprintf(os, fault ? fault->name() : "NoFault");
+}
+};
+
 struct AddrOp
 {
 struct ArgType
diff --git a/src/arch/x86/insts/microspecop.hh  
b/src/arch/x86/insts/microspecop.hh

new file mode 100644
index 000..e47903c
--- /dev/null
+++ b/src/arch/x86/insts/microspecop.hh
@@ -0,0 +1,58 @@
+/*
+ * Copyright 2021 Google Inc.
+ *
+ * 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.
+ */
+
+#ifndef __ARCH_X86_INSTS_MICROSPECOP_HH__
+#define __ARCH_X86_INSTS_MICROSPECOP_HH__
+
+#include "arch/x86/insts/microop.hh"
+#include "cpu/exec_context.hh"
+
+namespace X86ISA
+{
+
+class MicroHalt : public InstOperands
+{
+  public:
+MicroHalt(ExtMachInst mach_inst, const char *inst_mnem,
+uint64_t set_flags) :
+InstOperands(mach_inst, "halt", inst_mnem,
+set_flags | (ULL(1) << StaticInst::IsNonSpeculative) |
+(ULL(1) << StaticInst::IsQuiesce),
+No_OpClass)
+{}
+
+Fault
+execute(ExecContext *xc, Trace::InstRecord *) const override
+{
+xc->tcBase()->suspend();
+return NoFault;
+}
+};
+
+} // namespace X86ISA
+
+#endif //__ARCH_X86_INSTS_MICROSPECOP_HH__
diff --git a/src/arch/x86/isa/includes.isa b/src/arch/x86/isa/includes.isa
index b5fedb2..070d82f 100644
--- a/src/arch/x86/isa/includes.isa
+++ b/src/arch/x86/isa/includes.isa
@@ -60,6 +60,7 @@
 #include "arch/x86/insts/microldstop.hh"
 #include "arch/x86/insts/micromediaop.hh"
 #include "arch/x86/insts/microregop.hh"
+#include "arch/x86/insts/microspecop.hh"
 #include "arch/x86/insts/static_inst.hh"
 #include "arch/x86/isa_traits.hh"
 #include "arch/x86/registers.hh"

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Use the new op bases for memory microops.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42346 )



Change subject: arch-x86: Use the new op bases for memory microops.
..

arch-x86: Use the new op bases for memory microops.

Change-Id: I73538b547093e6f872e085686ea164ef89527321
---
M src/arch/x86/SConscript
D src/arch/x86/insts/microldstop.cc
M src/arch/x86/insts/microldstop.hh
M src/arch/x86/insts/microop_args.hh
M src/arch/x86/isa/microops/ldstop.isa
M src/arch/x86/isa/operands.isa
6 files changed, 248 insertions(+), 249 deletions(-)



diff --git a/src/arch/x86/SConscript b/src/arch/x86/SConscript
index f790ec1..af53022 100644
--- a/src/arch/x86/SConscript
+++ b/src/arch/x86/SConscript
@@ -51,7 +51,6 @@
 Source('fs_workload.cc')
 Source('insts/badmicroop.cc')
 Source('insts/microfpop.cc')
-Source('insts/microldstop.cc')
 Source('insts/micromediaop.cc')
 Source('insts/microop.cc')
 Source('insts/microregop.cc')
diff --git a/src/arch/x86/insts/microldstop.cc  
b/src/arch/x86/insts/microldstop.cc

deleted file mode 100644
index 1debacc..000
--- a/src/arch/x86/insts/microldstop.cc
+++ /dev/null
@@ -1,78 +0,0 @@
-/*
- * Copyright (c) 2007 The Hewlett-Packard Development Company
- * Copyright (c) 2015 Advanced Micro Devices, Inc.
- * 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.
- */
-
-#include "arch/x86/insts/microldstop.hh"
-
-#include 
-
-namespace X86ISA
-{
-
-std::string
-LdStOp::generateDisassembly(Addr pc, const Loader::SymbolTable *symtab)  
const

-{
-std::stringstream response;
-
-printMnemonic(response, instMnem, mnemonic);
-if (flags[IsLoad])
-printDestReg(response, 0, dataSize);
-else
-printSrcReg(response, 2, dataSize);
-response << ", ";
-printMem(response, segment, scale, index, base, disp, addressSize,  
false);

-return response.str();
-}
-
-std::string
-LdStSplitOp::generateDisassembly(
-Addr pc, const Loader::SymbolTable *symtab) const
-{
-std::stringstream response;
-
-printMnemonic(response, instMnem, mnemonic);
-int baseRegIdx = flags[IsLoad] ? 0 : 2;
-response << "[";
-printDestReg(response, baseRegIdx, dataSize);
-response << ", ";
-printDestReg(response, baseRegIdx+1, dataSize);
-response << "], ";
-printMem(response, segment, scale, index, base, disp, addressSize,  
false);

-return response.str();
-}
-
-}
diff --git a/src/arch/x86/insts/microldstop.hh  
b/src/arch/x86/insts/microldstop.hh

index 2611095..f17b52f 100644
--- a/src/arch/x86/insts/microldstop.hh
+++ b/src/arch/x86/insts/microldstop.hh
@@ -40,6 +40,7 @@
 #define __ARCH_X86_INSTS_MICROLDSTOP_HH__

 #include "arch/x86/insts/microop.hh"
+#include "arch/x86/insts/microop_args.hh"
 #include "arch/x86/ldstflags.hh"
 #include "mem/packet.hh"
 #include "mem/request.hh"
@@ -54,65 +55,62 @@
 class MemOp : public X86MicroopBase
 {
   

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Use the *Op classes with FP microops.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42348 )



Change subject: arch-x86: Use the *Op classes with FP microops.
..

arch-x86: Use the *Op classes with FP microops.

Change-Id: I79e68ad5a0233047d44079d8453bf232cb64d27e
---
M src/arch/x86/SConscript
D src/arch/x86/insts/microfpop.cc
M src/arch/x86/insts/microfpop.hh
M src/arch/x86/insts/microop_args.hh
M src/arch/x86/isa/microops/fpop.isa
5 files changed, 135 insertions(+), 201 deletions(-)



diff --git a/src/arch/x86/SConscript b/src/arch/x86/SConscript
index af53022..20f02dd 100644
--- a/src/arch/x86/SConscript
+++ b/src/arch/x86/SConscript
@@ -50,7 +50,6 @@
 Source('faults.cc')
 Source('fs_workload.cc')
 Source('insts/badmicroop.cc')
-Source('insts/microfpop.cc')
 Source('insts/micromediaop.cc')
 Source('insts/microop.cc')
 Source('insts/microregop.cc')
diff --git a/src/arch/x86/insts/microfpop.cc  
b/src/arch/x86/insts/microfpop.cc

deleted file mode 100644
index 1a32b6a..000
--- a/src/arch/x86/insts/microfpop.cc
+++ /dev/null
@@ -1,62 +0,0 @@
-/*
- * Copyright (c) 2007 The Hewlett-Packard Development Company
- * 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.
- */
-
-#include "arch/x86/insts/microfpop.hh"
-
-#include 
-
-#include "arch/x86/regs/misc.hh"
-
-namespace X86ISA
-{
-
-std::string
-FpOp::generateDisassembly(
-Addr pc, const Loader::SymbolTable *symtab) const
-{
-std::stringstream response;
-
-printMnemonic(response, instMnem, mnemonic);
-printDestReg(response, 0, dataSize);
-response << ", ";
-printSrcReg(response, 0, dataSize);
-response << ", ";
-printSrcReg(response, 1, dataSize);
-return response.str();
-}
-
-}
diff --git a/src/arch/x86/insts/microfpop.hh  
b/src/arch/x86/insts/microfpop.hh

index e9d32da..4c2903d 100644
--- a/src/arch/x86/insts/microfpop.hh
+++ b/src/arch/x86/insts/microfpop.hh
@@ -43,33 +43,21 @@
 namespace X86ISA
 {

-/**
- * Base classes for FpOps which provides a generateDisassembly method.
- */
 class FpOp : public X86MicroopBase
 {
   protected:
-const RegIndex src1;
-const RegIndex src2;
-const RegIndex dest;
-const uint8_t dataSize;
 const int8_t spm;

 // Constructor
-FpOp(ExtMachInst _machInst,
-const char *mnem, const char *_instMnem,
-uint64_t setFlags,
-InstRegIndex _src1, InstRegIndex _src2, InstRegIndex _dest,
-uint8_t _dataSize, int8_t _spm,
-OpClass __opClass) :
-X86MicroopBase(_machInst, mnem, _instMnem, setFlags,
-__opClass),
-src1(_src1.index()), src2(_src2.index()), dest(_dest.index()),
-dataSize(_dataSize), spm(_spm)
+FpOp(ExtMachInst mach_inst, const char *mnem, const char *inst_mnem,
+uint64_t set_flags, OpClass op_class,
+uint8_t data_size, int8_t _spm) :

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Use the newly flexible RegOpT to implement the limm uop.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42349 )



Change subject: arch-x86: Use the newly flexible RegOpT to implement the  
limm uop.

..

arch-x86: Use the newly flexible RegOpT to implement the limm uop.

Change-Id: I7ac632d891b0f6b42794eb894bde3c18ce82718a
---
M src/arch/x86/insts/microop_args.hh
M src/arch/x86/isa/microops/limmop.isa
2 files changed, 45 insertions(+), 62 deletions(-)



diff --git a/src/arch/x86/insts/microop_args.hh  
b/src/arch/x86/insts/microop_args.hh

index d57ae12..a3ebe29 100644
--- a/src/arch/x86/insts/microop_args.hh
+++ b/src/arch/x86/insts/microop_args.hh
@@ -247,6 +247,22 @@
 }
 };

+struct Imm64Op
+{
+using ArgType = uint64_t;
+
+uint64_t imm64;
+
+template 
+Imm64Op(InstType *inst, ArgType _imm64) : imm64(_imm64) {}
+
+void
+print(std::ostream ) const
+{
+ccprintf(os, "%#x", imm64);
+}
+};
+
 struct AddrOp
 {
 struct ArgType
diff --git a/src/arch/x86/isa/microops/limmop.isa  
b/src/arch/x86/isa/microops/limmop.isa

index 51310b4..d6a909f 100644
--- a/src/arch/x86/isa/microops/limmop.isa
+++ b/src/arch/x86/isa/microops/limmop.isa
@@ -40,71 +40,40 @@
 //

 def template MicroLimmOpExecute {{
-Fault %(class_name)s::execute(ExecContext *xc,
-Trace::InstRecord *traceData) const
-{
-%(op_decl)s;
-%(op_rd)s;
-%(code)s;
-%(op_wb)s;
-return NoFault;
-}
+Fault
+%(class_name)s::execute(ExecContext *xc,
+Trace::InstRecord *traceData) const
+{
+%(op_decl)s;
+%(op_rd)s;
+%(code)s;
+%(op_wb)s;
+return NoFault;
+}
 }};

 def template MicroLimmOpDeclare {{
-class %(class_name)s : public X86ISA::X86MicroopBase
+class %(class_name)s : public %(base_class)s
 {
   private:
 %(reg_idx_arr_decl)s;

-  protected:
-const RegIndex dest;
-const uint64_t imm;
-const uint8_t dataSize;
-RegIndex foldOBit;
-
-std::string generateDisassembly(Addr pc,
-const Loader::SymbolTable *symtab) const override;
-
   public:
-%(class_name)s(ExtMachInst _machInst,
-const char * instMnem,
-uint64_t setFlags, InstRegIndex _dest,
-uint64_t _imm, uint8_t _dataSize);
+template 
+%(class_name)s(ExtMachInst mach_inst, const char *inst_mnem,
+uint64_t set_flags, Dest _dest,
+uint64_t _imm, uint8_t data_size) :
+%(base_class)s(mach_inst, "%(mnemonic)s", inst_mnem, set_flags,
+%(op_class)s, _dest, _imm, data_size, 0)
+{
+%(set_reg_idx_arr)s;
+%(constructor)s;
+}

 Fault execute(ExecContext *, Trace::InstRecord *) const override;
 };
 }};

-def template MicroLimmOpDisassembly {{
-std::string
-%(class_name)s::generateDisassembly(
-Addr pc, const Loader::SymbolTable *symtab) const
-{
-std::stringstream response;
-
-printMnemonic(response, instMnem, mnemonic);
-printDestReg(response, 0, dataSize);
-response << ", ";
-ccprintf(response, "%#x", imm);
-return response.str();
-}
-}};
-
-def template MicroLimmOpConstructor {{
-%(class_name)s::%(class_name)s(
-ExtMachInst machInst, const char * instMnem, uint64_t setFlags,
-InstRegIndex _dest, uint64_t _imm, uint8_t _dataSize) :
-%(base_class)s(machInst, "%(mnemonic)s", instMnem,
-setFlags, %(op_class)s),
-dest(_dest.index()), imm(_imm), dataSize(_dataSize)
-{
-%(set_reg_idx_arr)s;
-foldOBit = (dataSize == 1 && !machInst.rex.present) ? 1 << 6 : 0;
-%(constructor)s;
-}
-}};
-
 let {{
 class LimmOp(X86Microop):
 def __init__(self, dest, imm, dataSize="env.dataSize"):
@@ -162,21 +131,19 @@
 }};

 let {{
+base = 'X86ISA::RegOpT'
 # Build up the all register version of this micro op
-iops = [InstObjParams("limm", "Limm", 'X86MicroopBase',
-{"code" : "DestReg = merge(DestReg, imm, dataSize);"}),
-InstObjParams("limm", "LimmBig", 'X86MicroopBase',
-{"code" : "DestReg = imm & mask(dataSize * 8);"})]
+iops = [InstObjParams("limm", "Limm", base,
+{"code" : "DestReg = merge(DestReg, imm64, dataSize);"}),
+InstObjParams("limm", "LimmBig", base,
+{"code" : "DestReg = imm64 & mask(dataSize * 8);"})]
 for iop in iops:
 header_output += MicroLimmOpDeclare.subst(iop)
-decoder_output += MicroLimmOpConstructor.subst(iop)
-decoder_output += MicroLimmOpDisassembly.subst(iop)
 exec_output += MicroLimmOpExecute.subst(iop)

-iop = 

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Clean up some style issues in regop.isa.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42341 )



Change subject: arch-x86: Clean up some style issues in regop.isa.
..

arch-x86: Clean up some style issues in regop.isa.

Change-Id: Ied817adab4e6a3b0ae56e07138b0b2e23dd83892
---
M src/arch/x86/isa/microops/regop.isa
1 file changed, 100 insertions(+), 75 deletions(-)



diff --git a/src/arch/x86/isa/microops/regop.isa  
b/src/arch/x86/isa/microops/regop.isa

index 63a1683..871b0c7 100644
--- a/src/arch/x86/isa/microops/regop.isa
+++ b/src/arch/x86/isa/microops/regop.isa
@@ -40,64 +40,58 @@
 //

 def template MicroRegOpExecute {{
-Fault %(class_name)s::execute(ExecContext *xc,
-Trace::InstRecord *traceData) const
-{
-Fault fault = NoFault;
+Fault
+%(class_name)s::execute(ExecContext *xc,
+Trace::InstRecord *traceData) const
+{
+Fault fault = NoFault;

-DPRINTF(X86, "The data size is %d\n", dataSize);
-%(op_decl)s;
-%(op_rd)s;
+DPRINTF(X86, "The data size is %d\n", dataSize);
+%(op_decl)s;
+%(op_rd)s;

-M5_VAR_USED RegVal result;
+M5_VAR_USED RegVal result;

-if(%(cond_check)s)
-{
-%(code)s;
-%(flag_code)s;
-}
-else
-{
-%(else_code)s;
-}
-
-//Write the resulting state to the execution context
-if(fault == NoFault)
-{
-%(op_wb)s;
-}
-return fault;
+if (%(cond_check)s) {
+%(code)s;
+%(flag_code)s;
+} else {
+%(else_code)s;
 }
+
+//Write the resulting state to the execution context
+if (fault == NoFault) {
+%(op_wb)s;
+}
+return fault;
+}
 }};

 def template MicroRegOpImmExecute {{
-Fault %(class_name)s::execute(ExecContext *xc,
-Trace::InstRecord *traceData) const
-{
-Fault fault = NoFault;
+Fault
+%(class_name)s::execute(ExecContext *xc,
+Trace::InstRecord *traceData) const
+{
+Fault fault = NoFault;

-%(op_decl)s;
-%(op_rd)s;
+%(op_decl)s;
+%(op_rd)s;

-M5_VAR_USED RegVal result;
+M5_VAR_USED RegVal result;

-if(%(cond_check)s)
-{
-%(code)s;
-%(flag_code)s;
-}
-else
-{
-%(else_code)s;
-}
-
-//Write the resulting state to the execution context
-if(fault == NoFault)
-{
-%(op_wb)s;
-}
-return fault;
+if (%(cond_check)s) {
+%(code)s;
+%(flag_code)s;
+} else {
+%(else_code)s;
 }
+
+//Write the resulting state to the execution context
+if (fault == NoFault) {
+%(op_wb)s;
+}
+return fault;
+}
 }};

 def template MicroRegOpDeclare {{
@@ -114,8 +108,8 @@

 Fault execute(ExecContext *, Trace::InstRecord *) const override;

-X86ISA::PCState branchTarget(const X86ISA::PCState ) const
-override;
+X86ISA::PCState branchTarget(
+const X86ISA::PCState ) const override;

 /// Explicitly import the otherwise hidden branchTarget
 using StaticInst::branchTarget;
@@ -137,8 +131,8 @@

 Fault execute(ExecContext *, Trace::InstRecord *) const override;

-X86ISA::PCState branchTarget(const X86ISA::PCState ) const
-override;
+X86ISA::PCState branchTarget(
+const X86ISA::PCState ) const override;

 /// Explicitly import the otherwise hidden branchTarget
 using StaticInst::branchTarget;
@@ -147,7 +141,7 @@

 def template MicroRegOpConstructor {{
 %(class_name)s::%(class_name)s(
-ExtMachInst machInst, const char * instMnem, uint64_t setFlags,
+ExtMachInst machInst, const char *instMnem, uint64_t setFlags,
 InstRegIndex _src1, InstRegIndex _src2, InstRegIndex _dest,
 uint8_t _dataSize, uint16_t _ext) :
 %(base_class)s(machInst, "%(mnemonic)s", instMnem, setFlags,
@@ -164,8 +158,8 @@
 {
 X86ISA::PCState pcs = branchPC;
 DPRINTF(X86, "branchTarget PC info: %s, Immediate: %lx\n",
-pcs, (int64_t) this->machInst.immediate);
-pcs.npc(pcs.npc() + (int64_t) this->machInst.immediate);
+pcs, (int64_t)this->machInst.immediate);
+pcs.npc(pcs.npc() + (int64_t)this->machInst.immediate);
 pcs.uEnd();
 return pcs;
 }
@@ -173,12 +167,11 @@

 def 

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Remove static code from debug.isa and fix style.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42347 )



Change subject: arch-x86: Remove static code from debug.isa and fix style.
..

arch-x86: Remove static code from debug.isa and fix style.

Fix style problems in debug.isa, and move static code out of the ISA
description into a plain header file.

Change-Id: I8369bd8d46ad11b66cac8249cc981a8d279a1492
---
M src/arch/x86/insts/badmicroop.cc
A src/arch/x86/insts/microdebug.hh
M src/arch/x86/isa/includes.isa
M src/arch/x86/isa/microops/debug.isa
4 files changed, 90 insertions(+), 62 deletions(-)



diff --git a/src/arch/x86/insts/badmicroop.cc  
b/src/arch/x86/insts/badmicroop.cc

index 9b4d32a..d85a1b0 100644
--- a/src/arch/x86/insts/badmicroop.cc
+++ b/src/arch/x86/insts/badmicroop.cc
@@ -55,7 +55,7 @@
 // try to delete the static memory when it was destructed.

 const StaticInstPtr badMicroop =
-new X86ISAInst::MicroDebug(dummyMachInst, "panic", "BAD",
+new MicroDebug(dummyMachInst, "panic", "BAD",
 StaticInst::IsMicroop | StaticInst::IsLastMicroop,
 new GenericISA::M5PanicFault("Invalid microop!"));

diff --git a/src/arch/x86/insts/microdebug.hh  
b/src/arch/x86/insts/microdebug.hh

new file mode 100644
index 000..b3af1e7
--- /dev/null
+++ b/src/arch/x86/insts/microdebug.hh
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2021 Google Inc.
+ *
+ * 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.
+ */
+
+#ifndef __ARCH_X86_INSTS_MICRODEBUG_HH__
+#define __ARCH_X86_INSTS_MICRODEBUG_HH__
+
+#include "arch/x86/insts/microop.hh"
+
+namespace X86ISA
+{
+
+class MicroDebug : public X86ISA::X86MicroopBase
+{
+  protected:
+std::shared_ptr fault;
+
+  public:
+MicroDebug(ExtMachInst mach_inst, const char *mnem, const char  
*inst_mnem,

+uint64_t set_flags, GenericISA::M5DebugFault *_fault) :
+X86MicroopBase(mach_inst, mnem, inst_mnem, set_flags, No_OpClass),
+fault(_fault)
+{}
+
+Fault
+execute(ExecContext *xc, Trace::InstRecord *traceData) const override
+{
+return fault;
+}
+
+std::string
+generateDisassembly(Addr pc,
+const Loader::SymbolTable *symtab) const override
+{
+std::stringstream response;
+
+printMnemonic(response, instMnem, mnemonic);
+response << "\"" << fault->message() << "\"";
+
+return response.str();
+}
+};
+
+} // namespace X86ISA
+
+#endif //__ARCH_X86_INSTS_MICRODEBUG_HH__
diff --git a/src/arch/x86/isa/includes.isa b/src/arch/x86/isa/includes.isa
index 4b95655..b5fedb2 100644
--- a/src/arch/x86/isa/includes.isa
+++ b/src/arch/x86/isa/includes.isa
@@ -55,6 +55,7 @@
 #include "arch/generic/debugfaults.hh"
 #include "arch/x86/emulenv.hh"
 #include "arch/x86/insts/macroop.hh"
+#include "arch/x86/insts/microdebug.hh"
 #include "arch/x86/insts/microfpop.hh"
 #include "arch/x86/insts/microldstop.hh"
 #include "arch/x86/insts/micromediaop.hh"
diff --git a/src/arch/x86/isa/microops/debug.isa  
b/src/arch/x86/isa/microops/debug.isa

index 326f245..5023682 100644
--- a/src/arch/x86/isa/microops/debug.isa
+++ b/src/arch/x86/isa/microops/debug.isa
@@ -39,47 +39,6 @@
 //
 //

-output header {{
-class MicroDebug : public X86ISA::X86MicroopBase
-{
-  protected:
-std::shared_ptr fault;
-
-  public:
-MicroDebug(ExtMachInst _machInst, const char *mnem,
-

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Factor out duplication in the new RegOp base classes.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42344 )



Change subject: arch-x86: Factor out duplication in the new RegOp base  
classes.

..

arch-x86: Factor out duplication in the new RegOp base classes.

Instead of having the cross product of dest/src1/src2 and folded int,
debug, control, misc, and segment operands, break them up so they can be
mixed together in different combinations using "using" declarations.

Change-Id: I04357b08bd8a6cd91c2e4df64a2c6cb760bfe90e
---
M src/arch/x86/insts/microregop.hh
1 file changed, 90 insertions(+), 186 deletions(-)



diff --git a/src/arch/x86/insts/microregop.hh  
b/src/arch/x86/insts/microregop.hh

index e7bd053..37e9d6f 100644
--- a/src/arch/x86/insts/microregop.hh
+++ b/src/arch/x86/insts/microregop.hh
@@ -43,217 +43,121 @@
 namespace X86ISA
 {

-struct RegOpDest
+struct DestOp
 {
 using ArgType = InstRegIndex;
+const RegIndex dest;
+const size_t size;
+RegIndex index() const { return dest; }

-RegIndex dest;
-size_t size;
+DestOp(RegIndex _dest, size_t _size) : dest(_dest), size(_size) {}
+};

+struct Src1Op
+{
+using ArgType = InstRegIndex;
+const RegIndex src1;
+const size_t size;
+RegIndex index() const { return src1; }
+
+Src1Op(RegIndex _src1, size_t _size) : src1(_src1), size(_size) {}
+};
+
+struct Src2Op
+{
+using ArgType = InstRegIndex;
+const RegIndex src2;
+const size_t size;
+RegIndex index() const { return src2; }
+
+Src2Op(RegIndex _src2, size_t _size) : src2(_src2), size(_size) {}
+};
+
+template 
+struct FoldedOp : public Base
+{
 template 
-RegOpDest(InstType *inst, ArgType idx) :
-dest(INTREG_FOLDED(idx.index(), inst->foldOBit)),
-size(inst->dataSize)
+FoldedOp(InstType *inst, typename Base::ArgType idx) :
+Base(INTREG_FOLDED(idx.index(), inst->foldOBit), inst->dataSize)
 {}

 void
 print(std::ostream ) const
 {
-X86StaticInst::printReg(os, RegId(IntRegClass, dest), size);
+X86StaticInst::printReg(os, RegId(IntRegClass, this->index()),
+this->size);
 }
 };

-struct RegOpDbgDest
+template 
+struct CrOp : public Base
 {
-using ArgType = InstRegIndex;
-
-RegIndex dest;
-size_t size;
-
 template 
-RegOpDbgDest(InstType *inst, ArgType idx) : dest(idx.index()),
-size(inst->dataSize)
+CrOp(InstType *inst, typename Base::ArgType idx) : Base(idx.index(),  
0) {}

+
+void
+print(std::ostream ) const
+{
+ccprintf(os, "cr%d", this->index());
+}
+};
+
+template 
+struct DbgOp : public Base
+{
+template 
+DbgOp(InstType *inst, typename Base::ArgType idx) : Base(idx.index(),  
0) {}

+
+void
+print(std::ostream ) const
+{
+ccprintf(os, "dr%d", this->index());
+}
+
+};
+
+template 
+struct SegOp : public Base
+{
+template 
+SegOp(InstType *inst, typename Base::ArgType idx) : Base(idx.index(),  
0) {}

+
+void
+print(std::ostream ) const
+{
+X86StaticInst::printSegment(os, this->index());
+}
+};
+
+template 
+struct MiscOp : public Base
+{
+template 
+MiscOp(InstType *inst, typename Base::ArgType idx) :
+Base(idx.index(), inst->dataSize)
 {}

 void
 print(std::ostream ) const
 {
-ccprintf(os, "dr%d", dest);
+X86StaticInst::printReg(os, RegId(MiscRegClass, this->index()),
+this->size);
 }
 };

-struct RegOpCrDest
-{
-using ArgType = InstRegIndex;
+using RegOpDest = FoldedOp;
+using RegOpDbgDest = DbgOp;
+using RegOpCrDest = CrOp;
+using RegOpSegDest = SegOp;
+using RegOpMiscDest = MiscOp;

-RegIndex dest;
-size_t size;
+using RegOpSrc1 = FoldedOp;
+using RegOpDbgSrc1 = DbgOp;
+using RegOpCrSrc1 = CrOp;
+using RegOpSegSrc1 = SegOp;
+using RegOpMiscSrc1 = MiscOp;

-template 
-RegOpCrDest(InstType *inst, ArgType idx) : dest(idx.index()),
-size(inst->dataSize)
-{}
-
-void
-print(std::ostream ) const
-{
-ccprintf(os, "cr%d", dest);
-}
-};
-
-struct RegOpSegDest
-{
-using ArgType = InstRegIndex;
-
-RegIndex dest;
-size_t size;
-
-template 
-RegOpSegDest(InstType *inst, ArgType idx) : dest(idx.index()),
-size(inst->dataSize)
-{}
-
-void
-print(std::ostream ) const
-{
-X86StaticInst::printSegment(os, dest);
-}
-};
-
-struct RegOpMiscDest
-{
-using ArgType = InstRegIndex;
-
-RegIndex dest;
-size_t size;
-
-template 
-RegOpMiscDest(InstType *inst, ArgType idx) : dest(idx.index()),
-size(inst->dataSize)
-{}
-
-void
-print(std::ostream ) const
-{
-X86StaticInst::printReg(os, RegId(MiscRegClass, dest), size);
-}
-};
-
-struct RegOpSrc1
-{
-using ArgType = InstRegIndex;
-
-RegIndex src1;
-size_t size;
-
-template 
-

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Fix the "index" value for SSrcReg2.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42342 )



Change subject: arch-x86: Fix the "index" value for SSrcReg2.
..

arch-x86: Fix the "index" value for SSrcReg2.

This was set to 1, the same as SSrcReg1. That value is used to order the
registers in the source operand array. Other code then expects to find
operands in that order when, for example, looking up an index to pick
sub-parts of a register out, or to print a register name.

Since the index value of SSrcReg1 and SSrcReg2 were the same, they
wouldn't be sorted in a predictable way, meaning the code looking for
SSrcReg2's index might have found SSrcReg1's index instead and done the
wrong thing.

Change-Id: I75045e64595e249802f57d22023a7eeb7b8ac5c6
---
M src/arch/x86/isa/operands.isa
1 file changed, 1 insertion(+), 1 deletion(-)



diff --git a/src/arch/x86/isa/operands.isa b/src/arch/x86/isa/operands.isa
index 504deb7..a1a8396 100644
--- a/src/arch/x86/isa/operands.isa
+++ b/src/arch/x86/isa/operands.isa
@@ -90,7 +90,7 @@
 'SrcReg1':   foldInt('src1', 'foldOBit', 1),
 'SSrcReg1':  intReg('src1', 1),
 'SrcReg2':   foldInt('src2', 'foldOBit', 2),
-'SSrcReg2':  intReg('src2', 1),
+'SSrcReg2':  intReg('src2', 2),
 'Index': foldInt('index', 'foldABit', 3),
 'Base':  foldInt('base', 'foldABit', 4),
 'DestReg':   foldInt('dest', 'foldOBit', 5),

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42342
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: I75045e64595e249802f57d22023a7eeb7b8ac5c6
Gerrit-Change-Number: 42342
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

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Generalize the RegOp operands.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42345 )



Change subject: arch-x86: Generalize the RegOp operands.
..

arch-x86: Generalize the RegOp operands.

This mechanism can now be used in other types of microops.

Change-Id: I82cb15b9d7b3c1e684aaa7482ea98b313f1d85d9
---
A src/arch/x86/insts/microop_args.hh
M src/arch/x86/insts/microregop.hh
M src/arch/x86/isa/microops/regop.isa
3 files changed, 223 insertions(+), 172 deletions(-)



diff --git a/src/arch/x86/insts/microop_args.hh  
b/src/arch/x86/insts/microop_args.hh

new file mode 100644
index 000..27af296
--- /dev/null
+++ b/src/arch/x86/insts/microop_args.hh
@@ -0,0 +1,203 @@
+/*
+ * Copyright 2021 Google Inc.
+ *
+ * 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.
+ */
+
+#ifndef __ARCH_X86_INSTS_MICROOP_ARGS_HH__
+#define __ARCH_X86_INSTS_MICROOP_ARGS_HH__
+
+#include 
+#include 
+#include 
+
+#include "arch/x86/insts/static_inst.hh"
+#include "arch/x86/regs/int.hh"
+#include "arch/x86/types.hh"
+#include "base/cprintf.hh"
+#include "cpu/reg_class.hh"
+
+namespace X86ISA
+{
+
+struct DestOp
+{
+using ArgType = InstRegIndex;
+const RegIndex dest;
+const size_t size;
+RegIndex index() const { return dest; }
+
+DestOp(RegIndex _dest, size_t _size) : dest(_dest), size(_size) {}
+};
+
+struct Src1Op
+{
+using ArgType = InstRegIndex;
+const RegIndex src1;
+const size_t size;
+RegIndex index() const { return src1; }
+
+Src1Op(RegIndex _src1, size_t _size) : src1(_src1), size(_size) {}
+};
+
+struct Src2Op
+{
+using ArgType = InstRegIndex;
+const RegIndex src2;
+const size_t size;
+RegIndex index() const { return src2; }
+
+Src2Op(RegIndex _src2, size_t _size) : src2(_src2), size(_size) {}
+};
+
+template 
+struct FoldedOp : public Base
+{
+template 
+FoldedOp(InstType *inst, typename Base::ArgType idx) :
+Base(INTREG_FOLDED(idx.index(), inst->foldOBit), inst->dataSize)
+{}
+
+void
+print(std::ostream ) const
+{
+X86StaticInst::printReg(os, RegId(IntRegClass, this->index()),
+this->size);
+}
+};
+
+template 
+struct CrOp : public Base
+{
+template 
+CrOp(InstType *inst, typename Base::ArgType idx) : Base(idx.index(),  
0) {}

+
+void
+print(std::ostream ) const
+{
+ccprintf(os, "cr%d", this->index());
+}
+};
+
+template 
+struct DbgOp : public Base
+{
+template 
+DbgOp(InstType *inst, typename Base::ArgType idx) : Base(idx.index(),  
0) {}

+
+void
+print(std::ostream ) const
+{
+ccprintf(os, "dr%d", this->index());
+}
+
+};
+
+template 
+struct SegOp : public Base
+{
+template 
+SegOp(InstType *inst, typename Base::ArgType idx) : Base(idx.index(),  
0) {}

+
+void
+print(std::ostream ) const
+{
+X86StaticInst::printSegment(os, this->index());
+}
+};
+
+template 
+struct MiscOp : public Base
+{
+template 
+MiscOp(InstType *inst, typename Base::ArgType idx) :
+Base(idx.index(), inst->dataSize)
+{}
+
+void
+print(std::ostream ) const
+{
+X86StaticInst::printReg(os, RegId(MiscRegClass, this->index()),
+this->size);
+}
+};
+
+using FoldedDestOp = FoldedOp;
+using DbgDestOp = DbgOp;
+using CrDestOp = CrOp;
+using SegDestOp = SegOp;
+using MiscDestOp = MiscOp;
+
+using FoldedSrc1Op = FoldedOp;
+using DbgSrc1Op = DbgOp;
+using CrSrc1Op 

[gem5-dev] Change in gem5/gem5[develop]: arch-sparc: Fix an operator precedence bug in the iob device.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40955 )


Change subject: arch-sparc: Fix an operator precedence bug in the iob  
device.

..

arch-sparc: Fix an operator precedence bug in the iob device.

Like in the nomali library, this bug is in some code making a bitmask
where what bits are enabled depends on some conditions. It used ?: to
evaluate the conditions and | to aggregate the bits, but didn't use any
()s, so the | happened first, then the ?:s. This would generate an
incorrect bitmask.

Change-Id: Iabcc8a9fd38cde5de3c0627a3b143407247c0c0e
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40955
Maintainer: Gabe Black 
Tested-by: kokoro 
Reviewed-by: Boris Shingarov 
---
M src/dev/sparc/iob.cc
1 file changed, 2 insertions(+), 2 deletions(-)

Approvals:
  Boris Shingarov: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/dev/sparc/iob.cc b/src/dev/sparc/iob.cc
index a0d1982..624563e 100644
--- a/src/dev/sparc/iob.cc
+++ b/src/dev/sparc/iob.cc
@@ -101,8 +101,8 @@

 if (accessAddr >= IntCtlAddr && accessAddr < IntCtlAddr +  
IntCtlSize) {

 int index = (accessAddr - IntCtlAddr) >> 3;
-uint64_t data = intCtl[index].mask  ? 1 << 2 : 0 |
-intCtl[index].pend  ? 1 << 0 : 0;
+uint64_t data = (intCtl[index].mask  ? (1 << 2) : 0) |
+(intCtl[index].pend  ? (1 << 0) : 0);
 pkt->setBE(data);
 return;
 }



10 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40955
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: Iabcc8a9fd38cde5de3c0627a3b143407247c0c0e
Gerrit-Change-Number: 40955
Gerrit-PatchSet: 12
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Boris Shingarov 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
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]: arch-mips: Fix a bug in the MIPS yield instruction.

2021-03-05 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40957 )


Change subject: arch-mips: Fix a bug in the MIPS yield instruction.
..

arch-mips: Fix a bug in the MIPS yield instruction.

The yieldThread function implements MIPS's yield instruction, and had a
if condition in it, (src_reg && !yield_mask != 0), which upset clang. When
originally committed, this check read (src_reg & !yield_mask != 0), but
apparently as part of a cleanup sweep a long time ago, it was assumed
that the & was being used as a logical operator and was turned into &&.

Reading the actual description of what the yield instruction is supposed
to do, if src_reg is positive (it is at this point in the function),
then it's supposed to be treated as a bitvector. The YQMask register,
what gets passed in as yield_mask, can have bits set in it which mask
bits that might be set in src_reg, and if any are still set, the an
interrupt should happen, as implemented by the body of the if.

From this description, it's apparent that what the original code was
*trying* to do was to use yield_mask to mask any set bits in src_reg,
and then if any bits were left go into the body. The original author
used ! as a bitwise negating operator since what they *wanted* to do was
to block any bits in src_reg where yield_mask *is* set, and let through
any where yield_mask *is not* set. The & would do that, but only with a
bitwise negated yield_mask. Hence:

if ((src_reg & ~yield_mask) != 0) {
...
}

Change-Id: I30d0a47992750adf78c8aa0c28217da187e0cbda
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40957
Maintainer: Gabe Black 
Tested-by: kokoro 
Reviewed-by: Boris Shingarov 
---
M src/arch/mips/mt.hh
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Boris Shingarov: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/mips/mt.hh b/src/arch/mips/mt.hh
index 9ab3290..da4c51a 100644
--- a/src/arch/mips/mt.hh
+++ b/src/arch/mips/mt.hh
@@ -273,7 +273,7 @@
 curTick(), tc->threadId());
 }
 } else if (src_reg > 0) {
-if (src_reg && !yield_mask != 0) {
+if ((src_reg & ~yield_mask) != 0) {
 VPEControlReg vpeControl =  
tc->readMiscReg(MISCREG_VPE_CONTROL);

 vpeControl.excpt = 2;
 tc->setMiscReg(MISCREG_VPE_CONTROL, vpeControl);



11 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40957
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: I30d0a47992750adf78c8aa0c28217da187e0cbda
Gerrit-Change-Number: 40957
Gerrit-PatchSet: 13
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Boris Shingarov 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
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] RFC: Support for multiple non consecutive ranges in the AbstractMemory class

2021-03-05 Thread Giacomo Travaglini via gem5-dev
Hi devs,

In the past days I have been thinking about providing support for multiple non 
consecutive ranges to the memory object (AbstractMemory).
I already have a working implementation; I just need to come up with a good 
python interface for it.

I have opened a JIRA ticket [1] with my proposal; feel free to add some 
comments to it or propose better alternatives

Kind Regards

Giacomo

[1]: https://gem5.atlassian.net/browse/GEM5-926
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
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[release-staging-v21-0]: configs: NVM missing the xor_low_bit argument in create_mem_intf

2021-03-05 Thread Giacomo Travaglini (Gerrit) via gem5-dev
Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42321 )



Change subject: configs: NVM missing the xor_low_bit argument in  
create_mem_intf

..

configs: NVM missing the xor_low_bit argument in create_mem_intf

Change-Id: Ie197cec1eaa82ca61a6bbb82c33307a16d779dbd
Signed-off-by: Giacomo Travaglini 
---
M configs/common/MemConfig.py
1 file changed, 1 insertion(+), 1 deletion(-)



diff --git a/configs/common/MemConfig.py b/configs/common/MemConfig.py
index 6e78be5..1806fdb 100644
--- a/configs/common/MemConfig.py
+++ b/configs/common/MemConfig.py
@@ -241,7 +241,7 @@

 elif opt_nvm_type and (not opt_mem_type or range_iter % 2 ==  
0):

 nvm_intf = create_mem_intf(n_intf, r, i, nbr_mem_ctrls,
-   intlv_bits, intlv_size)
+intlv_bits, intlv_size, opt_xor_low_bit)
 # Set the number of ranks based on the command-line
 # options if it was explicitly set
 if issubclass(n_intf, m5.objects.NVMInterface) and \

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


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-0
Gerrit-Change-Id: Ie197cec1eaa82ca61a6bbb82c33307a16d779dbd
Gerrit-Change-Number: 42321
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini 
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

[gem5-dev] Change in gem5/gem5[develop]: cpu-kvm: refactor x86 fixup before kvm_run

2021-03-05 Thread Yu-hsin Wang (Gerrit) via gem5-dev
Yu-hsin Wang has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42301 )



Change subject: cpu-kvm: refactor x86 fixup before kvm_run
..

cpu-kvm: refactor x86 fixup before kvm_run

Since kvmRun still does lots of thing before really going to KVM, to
make the fixup more precise, I change ioctlRun to a virtual function and
make the derived class overrides it if needed.

Change-Id: Ifd75decf0a5445a5266398caebd8aac1a5e80c50
---
M src/cpu/kvm/base.hh
M src/cpu/kvm/x86_cpu.cc
M src/cpu/kvm/x86_cpu.hh
3 files changed, 25 insertions(+), 28 deletions(-)



diff --git a/src/cpu/kvm/base.hh b/src/cpu/kvm/base.hh
index d97845b..68eda46 100644
--- a/src/cpu/kvm/base.hh
+++ b/src/cpu/kvm/base.hh
@@ -569,6 +569,8 @@
 }
 /** @} */

+/** Execute the KVM_RUN ioctl */
+virtual void ioctlRun();

 /**
  * KVM memory port.  Uses default RequestPort behavior and provides an
@@ -678,9 +680,6 @@
 /** Try to drain the CPU if a drain is pending */
 bool tryDrain();

-/** Execute the KVM_RUN ioctl */
-void ioctlRun();
-
 /** KVM vCPU file descriptor */
 int vcpuFD;
 /** Size of MMAPed kvm_run area */
diff --git a/src/cpu/kvm/x86_cpu.cc b/src/cpu/kvm/x86_cpu.cc
index 4a7d21b..bae5a29 100644
--- a/src/cpu/kvm/x86_cpu.cc
+++ b/src/cpu/kvm/x86_cpu.cc
@@ -1225,7 +1225,7 @@
 if (_status == Idle)
 return 0;
 else
-return kvmRunWrapper(ticks);
+return BaseKvmCPU::kvmRun(ticks);
 }

 Tick
@@ -1245,33 +1245,14 @@
 // Limit the run to 1 millisecond. That is hopefully enough to
 // reach an interrupt window. Otherwise, we'll just try again
 // later.
-return kvmRunWrapper(1 * SimClock::Float::ms);
+return BaseKvmCPU::kvmRun(1 * SimClock::Float::ms);
 } else {
 DPRINTF(Drain, "kvmRunDrain: Delivering pending IO\n");

-return kvmRunWrapper(0);
+return BaseKvmCPU::kvmRun(0);
 }
 }

-Tick
-X86KvmCPU::kvmRunWrapper(Tick ticks)
-{
-struct kvm_run _run(*getKvmRunState());
-
-// Synchronize the APIC base and CR8 here since they are present
-// in the kvm_run struct, which makes the synchronization really
-// cheap.
-kvm_run.apic_base = tc->readMiscReg(MISCREG_APIC_BASE);
-kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
-
-const Tick run_ticks(BaseKvmCPU::kvmRun(ticks));
-
-tc->setMiscReg(MISCREG_APIC_BASE, kvm_run.apic_base);
-kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
-
-return run_ticks;
-}
-
 uint64_t
 X86KvmCPU::getHostCycles() const
 {
@@ -1404,6 +1385,23 @@
 return !pending_events;
 }

+void
+X86KvmCPU::ioctlRun()
+{
+struct kvm_run _run(*getKvmRunState());
+
+// Synchronize the APIC base and CR8 here since they are present
+// in the kvm_run struct, which makes the synchronization really
+// cheap.
+kvm_run.apic_base = tc->readMiscReg(MISCREG_APIC_BASE);
+kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
+
+BaseKvmCPU::ioctlRun();
+
+tc->setMiscReg(MISCREG_APIC_BASE, kvm_run.apic_base);
+kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
+}
+
 static struct kvm_cpuid_entry2
 makeKvmCpuid(uint32_t function, uint32_t index,
  CpuidResult )
diff --git a/src/cpu/kvm/x86_cpu.hh b/src/cpu/kvm/x86_cpu.hh
index a60d597..a114a8a 100644
--- a/src/cpu/kvm/x86_cpu.hh
+++ b/src/cpu/kvm/x86_cpu.hh
@@ -78,9 +78,6 @@
  */
 Tick kvmRunDrain() override;

-/** Wrapper that synchronizes state in kvm_run */
-Tick kvmRunWrapper(Tick ticks);
-
 uint64_t getHostCycles() const override;

 /**
@@ -158,6 +155,9 @@
  */
 bool archIsDrained() const override;

+/** Override for synchronizing state in kvm_run */
+void ioctlRun() override;
+
   private:
 /**
  * Support routines to update the state of the KVM CPU from gem5's

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/42301
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: Ifd75decf0a5445a5266398caebd8aac1a5e80c50
Gerrit-Change-Number: 42301
Gerrit-PatchSet: 1
Gerrit-Owner: Yu-hsin Wang 
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