[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Move the macroop type into pure python.

2022-02-04 Thread Gabe Black (Gerrit) via gem5-dev 
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56470 )



Change subject: arch-x86: Move the macroop type into pure python.
..

arch-x86: Move the macroop type into pure python.

Also expose it to the microcode assembler directly by making it part of
the ucasmlib namespace package.

Change-Id: I86e5193a35fc770363e6dbc6e2386880af312a1f
---
M src/arch/x86/isa/macroop.isa
M src/arch/x86/isa/microasm.isa
M src/arch/x86/ucasmlib/arch/x86/__init__.py
A src/arch/x86/ucasmlib/arch/x86/macroop.py
4 files changed, 228 insertions(+), 198 deletions(-)



diff --git a/src/arch/x86/isa/macroop.isa b/src/arch/x86/isa/macroop.isa
index bc5effc..8331d94 100644
--- a/src/arch/x86/isa/macroop.isa
+++ b/src/arch/x86/isa/macroop.isa
@@ -35,202 +35,6 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

-// Basic instruction class declaration template.
-def template MacroDeclare {{
-GEM5_DEPRECATED_NAMESPACE(X86Macroop, x86_macroop);
-namespace x86_macroop
-{
-/**
- * Static instruction class for "%(mnemonic)s".
- */
-class %(class_name)s : public %(base_class)s
-{
-  private:
-%(declareLabels)s
-  public:
-// Constructor.
-%(class_name)s(ExtMachInst machInst, X86ISA::EmulEnv _env);
-
-std::string generateDisassembly(
-Addr pc, const loader::SymbolTable *symtab) const  
override;

-};
-}
-}};
-
-def template MacroDisassembly {{
-std::string
-x86_macroop::%(class_name)s::generateDisassembly(
-Addr pc, const loader::SymbolTable *symtab) const
-{
-std::stringstream out;
-out << mnemonic << "\t";
-
-int regSize = %(regSize)s;
-%(disassembly)s
-// Shut up gcc.
-regSize = regSize;
-return out.str();
-}
-}};
-
-// Basic instruction class constructor template.
-def template MacroConstructor {{
-x86_macroop::%(class_name)s::%(class_name)s(
-ExtMachInst machInst, EmulEnv _env)
-: %(base_class)s("%(mnemonic)s", machInst, %(num_microops)s,  
_env)

-{
-%(adjust_env)s;
-%(adjust_imm)s;
-%(adjust_disp)s;
-%(init_env)s;
-%(constructor)s;
-const char *macrocodeBlock = "%(class_name)s";
-//alloc_microops is the code that sets up the microops
-//array in the parent class.
-%(alloc_microops)s;
-}
-}};
-
-let {{
-from ucasmlib.containers import Macroop
-class X86Macroop(Macroop):
-def setAdjustEnv(self, val):
-self.adjust_env = val
-def adjustImm(self, val):
-self.adjust_imm += val
-def adjustDisp(self, val):
-self.adjust_disp += val
-def serializeBefore(self):
-self.serialize_before = True
-def serializeAfter(self):
-self.serialize_after = True
-
-def function_call(self):
-self.function_call = True
-def function_return(self):
-self.function_return = True
-def control_direct(self):
-self.control_direct = True
-def control_indirect(self):
-self.control_indirect = True
-
-def __init__(self, name):
-super().__init__(name)
-self.directives = {
-"adjust_env" : self.setAdjustEnv,
-"adjust_imm" : self.adjustImm,
-"adjust_disp" : self.adjustDisp,
-"serialize_before" : self.serializeBefore,
-"serialize_after" : self.serializeAfter,
-"function_call" : self.function_call,
-"function_return" : self.function_return,
-"control_direct" : self.control_direct,
-"control_indirect" : self.control_indirect
-}
-self.declared = False
-self.adjust_env = ""
-self.init_env = ""
-self.adjust_imm = '''
-uint64_t adjustedImm = IMMEDIATE;
-//This is to pacify gcc in case the immediate isn't used.
-adjustedImm = adjustedImm;
-'''
-self.adjust_disp = '''
-uint64_t adjustedDisp = DISPLACEMENT;
-//This is to pacify gcc in case the displacement isn't  
used.

-adjustedDisp = adjustedDisp;
-'''
-self.serialize_before = False
-self.serialize_after = False
-self.function_call = False
-self.function_return = False
-self.control_direct = False
-self.control_indirect = False
-
-def getAllocator(self, env):
-return "new x86_macroop::%s(machInst, %s)

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Get rid of some cruft related to microcode.

2022-02-04 Thread Gabe Black (Gerrit) via gem5-dev 
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56466 )



Change subject: arch-x86: Get rid of some cruft related to microcode.
..

arch-x86: Get rid of some cruft related to microcode.

The x86 ISA description file for macroops had an Exec template in it
that wasn't used, and a base class for X86 macroops that can be folded
into the version defined in C++.

Change-Id: Id1b167b7b7e019e9e602fe758a091aa03a012b13
---
M src/arch/x86/insts/macroop.hh
M src/arch/x86/isa/macroop.isa
2 files changed, 24 insertions(+), 41 deletions(-)



diff --git a/src/arch/x86/insts/macroop.hh b/src/arch/x86/insts/macroop.hh
index 36718f7..c0fa53d 100644
--- a/src/arch/x86/insts/macroop.hh
+++ b/src/arch/x86/insts/macroop.hh
@@ -42,6 +42,7 @@
 #include "arch/x86/insts/static_inst.hh"
 #include "arch/x86/emulenv.hh"
 #include "arch/x86/types.hh"
+#include "base/logging.hh"

 namespace gem5
 {
@@ -84,6 +85,12 @@
 return microops[microPC];
 }

+Fault
+execute(ExecContext *, Trace::InstRecord *) const override
+{
+panic("Tried to execute macroop directly!");
+}
+
 std::string
 generateDisassembly(Addr pc,
 const loader::SymbolTable *symtab) const override
diff --git a/src/arch/x86/isa/macroop.isa b/src/arch/x86/isa/macroop.isa
index 6073756..9b6d682 100644
--- a/src/arch/x86/isa/macroop.isa
+++ b/src/arch/x86/isa/macroop.isa
@@ -35,45 +35,6 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

-//
-//
-//  Architecture independent
-//
-
-// Execute method for macroops.
-def template MacroExecPanic {{
-Fault
-execute(ExecContext *, Trace::InstRecord *) const override
-{
-panic("Tried to execute macroop directly!");
-return NoFault;
-}
-}};
-
-output header {{
-// Base class for combinationally generated macroops
-class Macroop : public X86ISA::MacroopBase
-{
-  public:
-Macroop(const char *mnem, ExtMachInst _machInst,
-uint32_t _numMicroops, X86ISA::EmulEnv _env)
-: MacroopBase(mnem, _machInst, _numMicroops, _env)
-{}
-
-Fault
-execute(ExecContext *, Trace::InstRecord *) const override
-{
-panic("Tried to execute macroop directly!");
-}
-};
-}};
-
-//
-//
-//  X86 specific
-//
-//
-
 // Basic instruction class declaration template.
 def template MacroDeclare {{
 GEM5_DEPRECATED_NAMESPACE(X86Macroop, x86_macroop);
@@ -200,7 +161,8 @@
 for (label, micropc) in self.labels.items():
 declareLabels += "const static uint64_t label_%s = %d;\n" \
   % (label, micropc)
-iop = InstObjParams(self.getMnemonic(), self.name, "Macroop",
+iop = InstObjParams(self.getMnemonic(), self.name,
+"X86ISA::MacroopBase",
 {"code" : "",
  "declareLabels" : declareLabels
 })
@@ -255,7 +217,8 @@
 regSize = '''(%s || (env.base == INTREG_RSP && %s) ?
  env.stackSize :
  env.dataSize)''' % (useStackSize, memoryInst)
-iop = InstObjParams(self.getMnemonic(), self.name, "Macroop",
+iop = InstObjParams(self.getMnemonic(), self.name,
+"X86ISA::MacroopBase",
 {"code" : "", "num_microops" : numMicroops,
  "alloc_microops" : allocMicroops,
  "adjust_env" : self.adjust_env,

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/56466
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: Id1b167b7b7e019e9e602fe758a091aa03a012b13
Gerrit-Change-Number: 56466
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: Add an arch/x86 subpackage to the ucasmlib namespace.

2022-02-04 Thread Gabe Black (Gerrit) via gem5-dev 
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56469 )



Change subject: arch-x86: Add an arch/x86 subpackage to the ucasmlib  
namespace.

..

arch-x86: Add an arch/x86 subpackage to the ucasmlib namespace.

This currently only has the ROM class specialization for x86.

Change-Id: Ic9e071109fa5b4d2f0e29502b7811ef777c0c4af
---
M src/arch/x86/isa/microasm.isa
D src/arch/x86/isa/rom.isa
A src/arch/x86/ucasmlib/arch/x86/__init__.py
A src/arch/x86/ucasmlib/arch/x86/rom.py
4 files changed, 92 insertions(+), 55 deletions(-)



diff --git a/src/arch/x86/isa/microasm.isa b/src/arch/x86/isa/microasm.isa
index ec9ed5a..5293410 100644
--- a/src/arch/x86/isa/microasm.isa
+++ b/src/arch/x86/isa/microasm.isa
@@ -44,16 +44,18 @@
 //Include code to build macroops in both C++ and python.
 ##include "macroop.isa"

-//Include code to fill out the microcode ROM in both C++ and python.
-##include "rom.isa"
-
 let {{
 import sys
+
+old_path = sys.path
 sys.path[0:0] = ["src/arch/x86/isa/"]
 from insts import microcode
-# print microcode
+sys.path[0:0] = ["src/arch/x86/"]
 from ucasmlib.assembler import MicroAssembler
-mainRom = X86MicrocodeRom('main ROM')
+from ucasmlib.arch.x86 import Rom
+sys.path = old_path
+
+mainRom = Rom('main ROM')
 assembler = MicroAssembler(X86Macroop, microopClasses, mainRom)

 def gpRegIdx(idx):
diff --git a/src/arch/x86/isa/rom.isa b/src/arch/x86/isa/rom.isa
deleted file mode 100644
index 58fd83c..000
--- a/src/arch/x86/isa/rom.isa
+++ /dev/null
@@ -1,50 +0,0 @@
-// Copyright (c) 2008 The Regents of The University of Michigan
-// All rights reserved.
-//
-// 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.
-
-let {{
-from ucasmlib.containers import Rom
-
-class X86MicrocodeRom(Rom):
-def getDeclaration(self):
-declareLabels = "namespace rom_labels\n{\n"
-for (label, micropc) in self.labels.items():
-declareLabels += "const static uint64_t label_%s = %d;\n" \
-  % (label, micropc)
-for (label, micropc) in self.externs.items():
-declareLabels += \
-"const static MicroPC extern_label_%s = %d;\n" \
-% (label, micropc)
-declareLabels += "}\n"
-return declareLabels;
-
-def getDefinition(self):
-elements = map(lambda op: op.getGenerator(), self.microops)
-allocator = "{" + ',\n'.join(elements) + "}"
-
-return '''
-X86ISA::MicrocodeRom::MicrocodeRom() : genFuncs(%s) {}
-''' % allocator
-}};
diff --git a/src/arch/x86/ucasmlib/arch/x86/__init__.py  
b/src/arch/x86/ucasmlib/arch/x86/__init__.py

new file mode 100644
index 000..6a1bdd7
--- /dev/null
+++ b/src/arch/x86/ucasmlib/arch/x86/__init__.py
@@ -0,0 +1,26 @@
+# Copyright 2022 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

[gem5-dev] Change in gem5/gem5[develop]: arch: Make ucasmlib a namespace package.

2022-02-04 Thread Gabe Black (Gerrit) via gem5-dev 
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56468 )



Change subject: arch: Make ucasmlib a namespace package.
..

arch: Make ucasmlib a namespace package.

This will let the ISAs define their own ucasmlib directories which will
let them extend the base microcode assembler to add their own
implementations/libraries of microops by setting the python path. The
implementations will live in the existing arch directories which will (I
hope) keep the organization of the source tree clean.

Change-Id: Ic5d25e11e22735ba65725cc360da133ca8758f57
---
M src/arch/micro_asm_test.py
R src/arch/ucasmlib/containers.py
M src/arch/x86/isa/macroop.isa
M src/arch/x86/isa/rom.isa
4 files changed, 18 insertions(+), 3 deletions(-)



diff --git a/src/arch/micro_asm_test.py b/src/arch/micro_asm_test.py
index d34f988..55f7b8e 100755
--- a/src/arch/micro_asm_test.py
+++ b/src/arch/micro_asm_test.py
@@ -24,7 +24,7 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

-from ucasmlib import Macroop, Rom
+from ucasmlib.containers import Macroop, Rom
 from ucasmlib.assembler import MicroAssembler

 class Bah(object):
diff --git a/src/arch/ucasmlib/__init__.py b/src/arch/ucasmlib/containers.py
similarity index 100%
rename from src/arch/ucasmlib/__init__.py
rename to src/arch/ucasmlib/containers.py
diff --git a/src/arch/x86/isa/macroop.isa b/src/arch/x86/isa/macroop.isa
index 59d7958..bc5effc 100644
--- a/src/arch/x86/isa/macroop.isa
+++ b/src/arch/x86/isa/macroop.isa
@@ -92,7 +92,7 @@
 }};

 let {{
-from ucasmlib import Macroop
+from ucasmlib.containers import Macroop
 class X86Macroop(Macroop):
 def setAdjustEnv(self, val):
 self.adjust_env = val
diff --git a/src/arch/x86/isa/rom.isa b/src/arch/x86/isa/rom.isa
index 289efc8..58fd83c 100644
--- a/src/arch/x86/isa/rom.isa
+++ b/src/arch/x86/isa/rom.isa
@@ -25,7 +25,7 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

 let {{
-from ucasmlib import Rom
+from ucasmlib.containers import Rom

 class X86MicrocodeRom(Rom):
 def getDeclaration(self):

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/56468
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: Ic5d25e11e22735ba65725cc360da133ca8758f57
Gerrit-Change-Number: 56468
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,arch-x86: Remove underdeveloped and unused RomMacroop.

2022-02-04 Thread Gabe Black (Gerrit) via gem5-dev 
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56467 )



Change subject: arch,arch-x86: Remove underdeveloped and unused RomMacroop.
..

arch,arch-x86: Remove underdeveloped and unused RomMacroop.

This was originally intended to be for macroops that didn't want to do
anything locally and jump directly into the microcode ROM, but it was
never used and incompletely developed.

Lets just get rid of it and reintroduce it later if it becomes
necessary, rather than have a half implementation floating around.

Change-Id: I2c644080c08f87e53dcc5009d00850ab342a6ef5
---
M src/arch/micro_asm_test.py
M src/arch/ucasmlib/__init__.py
M src/arch/ucasmlib/assembler.py
M src/arch/x86/isa/macroop.isa
M src/arch/x86/isa/microasm.isa
5 files changed, 24 insertions(+), 34 deletions(-)



diff --git a/src/arch/micro_asm_test.py b/src/arch/micro_asm_test.py
index b893736..d34f988 100755
--- a/src/arch/micro_asm_test.py
+++ b/src/arch/micro_asm_test.py
@@ -24,7 +24,7 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

-from ucasmlib import CombinationalMacroop, RomMacroop, Rom
+from ucasmlib import Macroop, Rom
 from ucasmlib.assembler import MicroAssembler

 class Bah(object):
@@ -51,7 +51,7 @@
 "dah": Dah
 }

-class TestMacroop(CombinationalMacroop):
+class TestMacroop(Macroop):
 def tweak(self):
 microops["bah"] = Bah_Tweaked
 def untweak(self):
@@ -67,7 +67,7 @@
 "print": self.print_debug
 }

-assembler = MicroAssembler(TestMacroop, microops, Rom('main ROM'),  
RomMacroop)

+assembler = MicroAssembler(TestMacroop, microops, Rom('main ROM'))

 testAssembly = '''
 # Single line comment
@@ -121,6 +121,5 @@
 bah
 };

-def macroop jumper (bar);
 '''
 assembler.assemble(testAssembly)
diff --git a/src/arch/ucasmlib/__init__.py b/src/arch/ucasmlib/__init__.py
index 983f813..f51455d 100644
--- a/src/arch/ucasmlib/__init__.py
+++ b/src/arch/ucasmlib/__init__.py
@@ -52,17 +52,9 @@
 string += "  %s\n" % microop
 return string

-class CombinationalMacroop(MicroContainer):
+class Macroop(MicroContainer):
 pass

-class RomMacroop:
-def __init__(self, name, target):
-self.name = name
-self.target = target
-
-def __str__(self):
-return "%s: %s\n" % (self.name, self.target)
-
 class Rom(MicroContainer):
 def __init__(self, name):
 super().__init__(name)
diff --git a/src/arch/ucasmlib/assembler.py b/src/arch/ucasmlib/assembler.py
index e2010df..00dfa9a 100644
--- a/src/arch/ucasmlib/assembler.py
+++ b/src/arch/ucasmlib/assembler.py
@@ -67,7 +67,6 @@

 'STRING',

-'LPAREN', 'RPAREN',
 'LBRACE', 'RBRACE',
 'SEMI', 'NEWLINE',
 )
@@ -227,8 +226,6 @@
 return t if t and t.type != 'eof' else None

 # Basic regular expressions to pick out simple tokens
-t_ANY_LPAREN = r'\('
-t_ANY_RPAREN = r'\)'
 t_ANY_SEMI = r';'

 t_ANY_ignore = ' \t\x0c'
@@ -308,18 +305,6 @@
 statement.handle(self, self.rom)
 t[0] = self.rom

-# Defines a macroop that jumps to an external label in the ROM.
-def p_macroop_jump(self, t):
-'macroop_def : DEF MACROOP ID LPAREN ID RPAREN SEMI'
-if not self.rom_macroop_type:
-self.print_error("ROM based macroop found, but no ROM macroop "
-"class was specified.")
-raise TypeError("ROM based macroop found, but no ROM macroop "
-"class was specified.")
-macroop = self.rom_macroop_type(t[3], t[5])
-self.macroops[t[3]] = macroop
-
-
 # Defines a macroop that is combinationally generated.
 def p_macroop_def(self, t):
 'macroop_def : DEF MACROOP ID block SEMI'
@@ -351,7 +336,7 @@
 def input(self, *args, **kwargs):
 return self.lexer.input(*args, **kwargs)

-def __init__(self, macro_type, microops, rom=None,  
rom_macroop_type=None):

+def __init__(self, macro_type, microops, rom=None):
 # Set lexers to something so when lex.lex() scans for doc strings  
the

 # "lexer" property can return something.
 self.lexers = [('dummy', None)]
@@ -362,7 +347,6 @@
 self.macroops = {}
 self.microops = microops
 self.rom = rom
-self.rom_macroop_type = rom_macroop_type
 self.symbols = {}

 def assemble(self, asm):
diff --git a/src/arch/x86/isa/macroop.isa b/src/arch/x86/isa/macroop.isa
index 9b6d682..59d7958 100644
--- a/src/arch/x86/isa/macroop.isa
+++ b/src/arch/x86/isa/macroop.isa
@@ -92,8 +92,8 @@
 }};

 let {{
-from ucasmlib import CombinationalMacroop, RomMacroop
-class X86Macroop(CombinationalMacroop):
+from ucasmlib import Macroop
+class X86Macroop(Macroop):
 def setAdjustEnv(self, val):

[gem5-dev] Change in gem5/gem5[develop]: mem-ruby: Support partial DMA writes in MOESI_AMD_Base-dir

2022-02-04 Thread Matthew Poremba (Gerrit) via gem5-dev 
Matthew Poremba has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56449 )



Change subject: mem-ruby: Support partial DMA writes in MOESI_AMD_Base-dir
..

mem-ruby: Support partial DMA writes in MOESI_AMD_Base-dir

This stores the byte address for a DMA request in a TBE. When the
request is sent to memory, the byte address is passed as well. The
AbstractController handles the creation of a packet smaller than a
cacheline after this point.

Change-Id: Ie669906dcdbece5b2eb275adaf84237a067ef11c
---
M src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
1 file changed, 33 insertions(+), 16 deletions(-)



diff --git a/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm  
b/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm

index 41c09b1..b62854e 100644
--- a/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
+++ b/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
@@ -144,24 +144,25 @@
   }

   structure(TBE, desc="...") {
-State TBEState, desc="Transient state";
-DataBlock DataBlk,  desc="data for the block";
-bool Dirty, desc="Is the data dirty?";
-int NumPendingAcks,desc="num acks expected";
-MachineID OriginalRequestor,desc="Original Requestor";
+State TBEState,   desc="Transient state";
+DataBlock DataBlk,desc="data for the block";
+bool Dirty,   desc="Is the data dirty?";
+int NumPendingAcks,   desc="num acks expected";
+MachineID OriginalRequestor,  desc="Original Requestor";
 MachineID WTRequestor,desc="WT Requestor";
-bool Cached,desc="data hit in Cache";
-bool MemData,   desc="Got MemData?",default="false";
-bool wtData,   desc="Got write through data?",default="false";
-bool atomicData,   desc="Got Atomic op?",default="false";
-Cycles InitialRequestTime, desc="...";
-Cycles ForwardRequestTime, desc="...";
+bool Cached,  desc="data hit in Cache";
+bool MemData, desc="Got MemData?", default="false";
+bool wtData,  desc="Got write through data?",  
default="false";

+bool atomicData,  desc="Got Atomic op?", default="false";
+Cycles InitialRequestTime,desc="...";
+Cycles ForwardRequestTime,desc="...";
 Cycles ProbeRequestStartTime, desc="...";
-MachineID LastSender, desc="Mach which this block came from";
-bool L3Hit, default="false", desc="Was this an L3 hit?";
-uint64_t probe_id,desc="probe id for lifetime profiling";
-WriteMask writeMask,desc="outstanding write through mask";
-int Len,desc="Length of memory request for DMA";
+MachineID LastSender, desc="Mach which this block came from";
+bool L3Hit, default="false",  desc="Was this an L3 hit?";
+uint64_t probe_id,desc="probe id for lifetime profiling";
+WriteMask writeMask,  desc="outstanding write through mask";
+int Len,  desc="Length of memory request for DMA";
+Addr ByteAddr, default="0",   desc="Byte address for DMA";
   }

   structure(TBETable, external="yes") {
@@ -893,6 +894,7 @@
   tbe.NumPendingAcks := 0;
   tbe.Dirty := false;
   tbe.Len := in_msg.Len;
+  tbe.ByteAddr := in_msg.PhysicalAddress;
   if (in_msg.Type == DMARequestType:WRITE) {
 tbe.wtData := true;
 tbe.Dirty := true;
@@ -950,6 +952,7 @@
 out_msg.MessageSize := MessageSizeType:Writeback_Data;
 out_msg.DataBlk := tbe.DataBlk;
 out_msg.Len := tbe.Len;
+out_msg.ByteAddr := tbe.ByteAddr;
 DPRINTF(ProtocolTrace, "%s\n", out_msg);
   }
 }

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/56449
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: Ie669906dcdbece5b2eb275adaf84237a067ef11c
Gerrit-Change-Number: 56449
Gerrit-PatchSet: 1
Gerrit-Owner: Matthew Poremba 
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]: mem-ruby: Add protocol prints to MOESI_AMD_BASE-dma

2022-02-04 Thread Matthew Poremba (Gerrit) via gem5-dev 
Matthew Poremba has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56447 )



Change subject: mem-ruby: Add protocol prints to MOESI_AMD_BASE-dma
..

mem-ruby: Add protocol prints to MOESI_AMD_BASE-dma

Change-Id: I59ed7311a8dc2a06ce1df0027891ba8e24e8a89e
---
M src/mem/ruby/protocol/MOESI_AMD_Base-dma.sm
1 file changed, 11 insertions(+), 0 deletions(-)



diff --git a/src/mem/ruby/protocol/MOESI_AMD_Base-dma.sm  
b/src/mem/ruby/protocol/MOESI_AMD_Base-dma.sm

index dbecabd..993387d 100644
--- a/src/mem/ruby/protocol/MOESI_AMD_Base-dma.sm
+++ b/src/mem/ruby/protocol/MOESI_AMD_Base-dma.sm
@@ -139,6 +139,7 @@
 out_msg.Len := in_msg.Len;
 out_msg.Destination.add(mapAddressToMachine(address,  
MachineType:Directory));

 out_msg.MessageSize := MessageSizeType:Writeback_Control;
+DPRINTF(ProtocolTrace, "%s\n", out_msg);
   }
 }
   }
@@ -154,6 +155,7 @@
   out_msg.Len := in_msg.Len;
   out_msg.Destination.add(mapAddressToMachine(address,  
MachineType:Directory));

   out_msg.MessageSize := MessageSizeType:Writeback_Control;
+  DPRINTF(ProtocolTrace, "%s\n", out_msg);
 }
   }
   }

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/56447
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: I59ed7311a8dc2a06ce1df0027891ba8e24e8a89e
Gerrit-Change-Number: 56447
Gerrit-PatchSet: 1
Gerrit-Owner: Matthew Poremba 
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]: mem-ruby: Remove DirectoryMemory storage in MOESI_AMD_BASE-dir

2022-02-04 Thread Matthew Poremba (Gerrit) via gem5-dev 
Matthew Poremba has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56446 )



Change subject: mem-ruby: Remove DirectoryMemory storage in  
MOESI_AMD_BASE-dir

..

mem-ruby: Remove DirectoryMemory storage in MOESI_AMD_BASE-dir

This protocol is using an old style where read/writes to memory were
being done by writing to a DataBlock in a DirectoryMemory entry. This
results in having multiple copies of memory, leads to stale copies in at
least one memory (usually DRAM), and require --access-backing-store in
most cases to work properly. This changeset removes all references to
getDirectoryEntry(...).DataBlk and instead forwards those reads and
writes to DRAM always.

This results in new transient states BL_WM, BDW_WM, and B_WM which are
blocked states waiting on memory acks indicating a write request is
complete. The appropriate transitions are updates to move to these new
states and stall states are updated to include them. DMA write ACK is
also moved to when the request is sent to memory, rather than when the
request is received.

Change-Id: Ic5bd6a8a8881d7df782e0f7eed8be9d873610e04
---
M src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
1 file changed, 118 insertions(+), 64 deletions(-)



diff --git a/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm  
b/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm

index 901a4b1..41c09b1 100644
--- a/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
+++ b/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
@@ -61,26 +61,29 @@
 {
   // STATES
   state_declaration(State, desc="Directory states",  
default="Directory_State_U") {

-U, AccessPermission:Backing_Store, desc="unblocked";
-BL, AccessPermission:Busy,  desc="got L3 WB request";
+U, AccessPermission:Backing_Store,  desc="unblocked";
 // BL is Busy because it's possible for the data only to be in the  
network
 // in the WB, L3 has sent it and gone on with its business in possibly  
I

 // state.
-BDR_M, AccessPermission:Backing_Store,  desc="DMA read, blocked  
waiting for memory";
-BS_M, AccessPermission:Backing_Store, desc="blocked  
waiting for memory";
-BM_M, AccessPermission:Backing_Store, desc="blocked  
waiting for memory";
-B_M, AccessPermission:Backing_Store, desc="blocked  
waiting for memory";
-BP, AccessPermission:Backing_Store, desc="blocked  
waiting for probes, no need for memory";
-BDR_PM, AccessPermission:Backing_Store, desc="DMA read, blocked  
waiting for probes and memory";
-BS_PM, AccessPermission:Backing_Store,desc="blocked  
waiting for probes and Memory";
-BM_PM, AccessPermission:Backing_Store,desc="blocked  
waiting for probes and Memory";
-B_PM, AccessPermission:Backing_Store,desc="blocked  
waiting for probes and Memory";
-BDW_P, AccessPermission:Backing_Store, desc="DMA write, blocked  
waiting for probes, no need for memory";
-BDR_Pm, AccessPermission:Backing_Store, desc="DMA read, blocked  
waiting for probes, already got memory";
-BS_Pm, AccessPermission:Backing_Store,desc="blocked  
waiting for probes, already got memory";
-BM_Pm, AccessPermission:Backing_Store,desc="blocked  
waiting for probes, already got memory";
-B_Pm, AccessPermission:Backing_Store,desc="blocked  
waiting for probes, already got memory";
-B, AccessPermission:Backing_Store,  desc="sent  
response, Blocked til ack";

+BL, AccessPermission:Busy,  desc="got L3 WB request";
+BL_WM, AccessPermission:Busy,   desc="writing L3 WB to  
memory, waiting for ack";
+BDR_M, AccessPermission:Backing_Store,  desc="DMA read, blocked  
waiting for memory";
+BS_M, AccessPermission:Backing_Store,   desc="blocked waiting for  
memory";
+BM_M, AccessPermission:Backing_Store,   desc="blocked waiting for  
memory";
+B_M, AccessPermission:Backing_Store,desc="blocked waiting for  
memory";
+BP, AccessPermission:Backing_Store, desc="blocked waiting for  
probes, no need for memory";
+BDR_PM, AccessPermission:Backing_Store, desc="DMA read, blocked  
waiting for probes and memory";
+BS_PM, AccessPermission:Backing_Store,  desc="blocked waiting for  
probes and Memory";
+BM_PM, AccessPermission:Backing_Store,  desc="blocked waiting for  
probes and Memory";
+B_PM, AccessPermission:Backing_Store,   desc="blocked waiting for  
probes and Memory";
+BDW_P, AccessPermission:Backing_Store,  desc="DMA write, blocked  
waiting for probes, no need for memory";
+BDW_WM, AccessPermission:Backing_Store, desc="DMA write, writing  
to memory, waiting for ack";
+BDR_Pm, AccessPermission:Backing_Store, desc="DMA read, blocked  
waiting for prob

[gem5-dev] Change in gem5/gem5[develop]: cpu: Only acquire needed tokens in PTL tester

2022-02-04 Thread Matthew Poremba (Gerrit) via gem5-dev 
Matthew Poremba has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56450 )



Change subject: cpu: Only acquire needed tokens in PTL tester
..

cpu: Only acquire needed tokens in PTL tester

The tester currently assumes that one token per lane is needed when
checking if an action is ready to be issued. When actually issuing
requests, it is possible that a memory location is not valid for various
reasons. This was not being considered when checking for tokens causing
the tester to acquire more tokens than requests sent. Since tokens are
returned when requests are coalesced, this was causing some tokens never
to be returned, eventually depleting the token pool and causing a
deadlock.

Add a new method which determines the number of tokens needed for an
action. This is called by both the ready check method and the method to
issue the action to ensure they are aligned.

Change-Id: Ic1af72085c3b077539eb3fd129331e776ebdffbc
---
M src/cpu/testers/gpu_ruby_test/tester_thread.cc
M src/cpu/testers/gpu_ruby_test/tester_thread.hh
2 files changed, 69 insertions(+), 6 deletions(-)



diff --git a/src/cpu/testers/gpu_ruby_test/tester_thread.cc  
b/src/cpu/testers/gpu_ruby_test/tester_thread.cc

index 2dfd54a..760f8c2 100644
--- a/src/cpu/testers/gpu_ruby_test/tester_thread.cc
+++ b/src/cpu/testers/gpu_ruby_test/tester_thread.cc
@@ -152,6 +152,37 @@
 episodeHistory.push_back(curEpisode);
 }

+int
+TesterThread::getTokensNeeded()
+{
+if (!tokenPort) {
+return 0;
+}
+
+int tokens_needed = 0;
+curAction = curEpisode->peekCurAction();
+
+switch(curAction->getType()) {
+case Episode::Action::Type::ATOMIC:
+tokens_needed = numLanes;
+break;
+case Episode::Action::Type::LOAD:
+case Episode::Action::Type::STORE:
+for (int lane = 0; lane < numLanes; ++lane) {
+Location loc = curAction->getLocation(lane);
+
+if (loc != AddressManager::INVALID_LOCATION && loc >= 0) {
+tokens_needed++;
+}
+}
+break;
+default:
+tokens_needed = 0;
+}
+
+return tokens_needed;
+}
+
 bool
 TesterThread::isNextActionReady()
 {
@@ -162,10 +193,13 @@

 // Only GPU wavefront threads have a token port. For all other  
types

 // of threads evaluate to true.
-bool haveTokens = tokenPort ? tokenPort->haveTokens(numLanes) :  
true;

+bool haveTokens = true;

 switch(curAction->getType()) {
 case Episode::Action::Type::ATOMIC:
+haveTokens = tokenPort ?
+tokenPort->haveTokens(getTokensNeeded()) : true;
+
 // an atomic action must wait for all previous requests
 // to complete
 if (pendingLdStCount == 0 &&
@@ -206,7 +240,7 @@
 assert(pendingAtomicCount == 0);

 // can't issue if there is a pending fence
-if (pendingFenceCount > 0 || !haveTokens) {
+if (pendingFenceCount > 0) {
 return false;
 }

@@ -215,7 +249,7 @@
 for (int lane = 0; lane < numLanes; ++lane) {
 Location loc = curAction->getLocation(lane);

-if (loc != AddressManager::INVALID_LOCATION) {
+if (loc != AddressManager::INVALID_LOCATION && loc >=  
0) {

 Addr addr = addrManager->getAddress(loc);

 if (outstandingLoads.find(addr) !=
@@ -237,6 +271,12 @@
 }
 }

+haveTokens = tokenPort ?
+tokenPort->haveTokens(getTokensNeeded()) : true;
+if (!haveTokens) {
+return false;
+}
+
 return true;
 default:
 panic("The tester got an invalid action\n");
@@ -250,7 +290,7 @@
 switch(curAction->getType()) {
 case Episode::Action::Type::ATOMIC:
 if (tokenPort) {
-tokenPort->acquireTokens(numLanes);
+tokenPort->acquireTokens(getTokensNeeded());
 }
 issueAtomicOps();
 break;
@@ -262,13 +302,13 @@
 break;
 case Episode::Action::Type::LOAD:
 if (tokenPort) {
-tokenPort->acquireTokens(numLanes);
+tokenPort->acquireTokens(getTokensNeeded());
 }
 issueLoadOps();
 break;
 case Episode::Action::Type::STORE:
 if (tokenPort) {
-tokenPort->acquireTokens(numLanes);
+tokenPort->acquireTokens(getTokensNeeded());
 }
 issueStoreOps();
 break;
diff --git a/src/cpu/testers/gpu_ruby_test/tester_thread.hh  
b/src/cpu/testers/gpu_r

[gem5-dev] Change in gem5/gem5[develop]: mem-ruby: Support partial DMA/memory writes

2022-02-04 Thread Matthew Poremba (Gerrit) via gem5-dev 
Matthew Poremba has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56448 )



Change subject: mem-ruby: Support partial DMA/memory writes
..

mem-ruby: Support partial DMA/memory writes

Partial cache line requests are requests to memory which are less than
the size of a cache line. These are used heavily in DMA requests from
the GPU model but the current support forces the address to align with
the cache line address, which eventually requests in the wrong data
being selected from the request.

This adds support by passing along the byte address of the request from
the directory to AbstractController. The AbstractController creates a
packet using the byte address rather than the line address. The line
address is restored before responding to SLICC code which operates on
line addresses.

Change-Id: I9827441a4c07a8e01d12b2e9b84880ab77f91002
JIRA: https://gem5.atlassian.net/browse/GEM5-564
---
M src/mem/ruby/protocol/RubySlicc_MemControl.sm
M src/mem/ruby/slicc_interface/AbstractController.cc
2 files changed, 30 insertions(+), 5 deletions(-)



diff --git a/src/mem/ruby/protocol/RubySlicc_MemControl.sm  
b/src/mem/ruby/protocol/RubySlicc_MemControl.sm

index e8517a4..e642769 100644
--- a/src/mem/ruby/protocol/RubySlicc_MemControl.sm
+++ b/src/mem/ruby/protocol/RubySlicc_MemControl.sm
@@ -63,7 +63,8 @@
 // Message to and from Memory Control

 structure(MemoryMsg, desc="...", interface="Message") {
-  Addr addr,  desc="Physical address for this request";
+  Addr addr,desc="Cache line address for this request";
+  Addr ByteAddr, default="0",   desc="Byte address for this request";
   MemoryRequestType Type,   desc="Type of memory request (MEMORY_READ  
or MEMORY_WB)";

   MachineID Sender, desc="What component sent the data";
   MachineID OriginalRequestorMachId, desc="What component originally  
requested";
diff --git a/src/mem/ruby/slicc_interface/AbstractController.cc  
b/src/mem/ruby/slicc_interface/AbstractController.cc

index 396b128..d9c1507 100644
--- a/src/mem/ruby/slicc_interface/AbstractController.cc
+++ b/src/mem/ruby/slicc_interface/AbstractController.cc
@@ -266,13 +266,15 @@
 req_size = mem_msg->m_Len;
 }

-RequestPtr req
-= std::make_shared(mem_msg->m_addr, req_size, 0, m_id);
+Addr req_addr = mem_msg->m_ByteAddr ? mem_msg->m_ByteAddr
+: mem_msg->m_addr;
+
+RequestPtr req = std::make_shared(req_addr, req_size, 0,  
m_id);

 PacketPtr pkt;
 if (mem_msg->getType() == MemoryRequestType_MEMORY_WB) {
 pkt = Packet::createWrite(req);
 pkt->allocate();
-pkt->setData(mem_msg->m_DataBlk.getData(getOffset(mem_msg->m_addr),
+pkt->setData(mem_msg->m_DataBlk.getData(getOffset(req_addr),
 req_size));
 } else if (mem_msg->getType() == MemoryRequestType_MEMORY_READ) {
 pkt = Packet::createRead(req);
@@ -370,7 +372,7 @@
 assert(pkt->isResponse());

 std::shared_ptr msg =  
std::make_shared(clockEdge());

-(*msg).m_addr = pkt->getAddr();
+(*msg).m_addr = makeLineAddress(pkt->getAddr());
 (*msg).m_Sender = m_machineID;

 SenderState *s = dynamic_cast(pkt->senderState);

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/56448
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: I9827441a4c07a8e01d12b2e9b84880ab77f91002
Gerrit-Change-Number: 56448
Gerrit-PatchSet: 1
Gerrit-Owner: Matthew Poremba 
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]: configs: Allow for no DMAs in Ruby GPU tester

2022-02-04 Thread Matthew Poremba (Gerrit) via gem5-dev 
Matthew Poremba has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56451 )



Change subject: configs: Allow for no DMAs in Ruby GPU tester
..

configs: Allow for no DMAs in Ruby GPU tester

If there are no DMA devices, we should not create any tester threads,
sequencers, or an IO crossbar.

Change-Id: I5762a70a064df5310e3f30d41bffc7800b53eb0b
---
M configs/example/ruby_gpu_random_test.py
1 file changed, 25 insertions(+), 7 deletions(-)



diff --git a/configs/example/ruby_gpu_random_test.py  
b/configs/example/ruby_gpu_random_test.py

index a5d9a49..be6d3f7 100644
--- a/configs/example/ruby_gpu_random_test.py
+++ b/configs/example/ruby_gpu_random_test.py
@@ -234,11 +234,15 @@
 #
 # Make generic DMA sequencer for Ruby to use
 #
-dma_devices = [TesterDma()] * n_DMAs
-system.piobus = IOXBar()
-for _, dma_device in enumerate(dma_devices):
-dma_device.pio = system.piobus.mem_side_ports
-system.dma_devices = dma_devices
+if n_DMAs > 0:
+dma_devices = [TesterDma()] * n_DMAs
+system.piobus = IOXBar()
+for _, dma_device in enumerate(dma_devices):
+dma_device.pio = system.piobus.mem_side_ports
+system.dma_devices = dma_devices
+else:
+# If no DMAs, we do not need a piobus
+system.piobus = None

 #
 # Create the Ruby system
@@ -250,7 +254,8 @@
 # size of system.cpu
 cpu_list = [ system.cpu ] * args.num_cpus
 Ruby.create_system(args, full_system = False,
-   system = system, dma_ports = system.dma_devices,
+   system = system,
+   dma_ports = system.dma_devices if n_DMAs > 0 else [],
cpus = cpu_list)

 #
@@ -275,7 +280,8 @@
 for i, ruby_port in enumerate(system.ruby._cpu_ports):
 ruby_port.no_retry_on_stall = True
 ruby_port.using_ruby_tester = True
-ruby_port.mem_request_port = system.piobus.cpu_side_ports
+if system.piobus is not None:
+ruby_port.mem_request_port = system.piobus.cpu_side_ports

 if i < n_CUs:
 tester.cu_vector_ports = ruby_port.in_ports

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/56451
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: I5762a70a064df5310e3f30d41bffc7800b53eb0b
Gerrit-Change-Number: 56451
Gerrit-PatchSet: 1
Gerrit-Owner: Matthew Poremba 
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] Re: gem5 Minor release v21.2.1: Candidate patches

2022-02-04 Thread Bobby Bruce via gem5-dev 
Dear all,

Haven't heard much, or anything, from this. I'm going to take this as a
quiet thumbs up from the community.

I'm planning to release v21.1.2 on Tuesday, so there are still a few more
days to protest, ask questions, propose other patches, etc.

Kind regards,
Bobby
--
Dr. Bobby R. Bruce
Room 3050,
Kemper Hall, UC Davis
Davis,
CA, 95616

web: https://www.bobbybruce.net


On Mon, Jan 31, 2022 at 6:09 PM Bobby Bruce  wrote:

> Dear all,
>
> In the last month, since v21.2 has been released, gem5 contributors have
> fixed bugs and added improvements which I believe should be included as
> part of the v21.2 release. I've therefore decided to bundle these patches
> as part of a gem5 minor release; v21.2.1.
>
> I've been keeping track of patches I believe should be included in a minor
> release here: https://gem5.atlassian.net/browse/GEM5-1140 and have since
> cherry-picked them onto the minor release branch here:
> https://gem5-review.googlesource.com/c/public/gem5/+/56251.
>
> I'd appreciate it if the original authors of these commits could approve
> these patches being included in the minor release.
>
> If you believe there are other patches that belong as part of the minor
> release (bug fixes, stability improvements, that kind of thing), then
> please let me know.
>
> I'd like to release it at the beginning of next week but that's, of
> course, contingent on this all getting reviewed in time.
>
> Kind regards,
> Bobby
> --
> Dr. Bobby R. Bruce
> Room 3050,
> Kemper Hall, UC Davis
> Davis,
> CA, 95616
>
> web: https://www.bobbybruce.net
>
___
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] Re: Kokoro failures

2022-02-04 Thread Bobby Bruce via gem5-dev 
Hey Giacomo,

For the first two patches I can't access the failing Kokoro due to re-runs
overwriting the log. In the last one,
https://gem5-review.googlesource.com/c/public/gem5/+/56303/1, I'm not
seeing a timeout issue. That appears to be my aforementioned issue, which
I've come to guess is due to a failure of Kokoro to download a needed
resource correctly (which has been known to happen from time-to-time,
unfortunately). Of the ones that pass I can see them completing in the 5 to
6 hour mark, which is what I've come to expect for a normal run. If
timeouts are happening, I suspect it's something to do with Kokoro running
slower or stalling? I'm not trying to shift the blame here but I can't see
any other explanation.

The Log4J checker code at the end is new to me:

```
[19:56:19] Loading Log4j Scanner Binary onto Build VM [19:56:21] Loading
Log4j Scanner Wrapper Script onto Build VM [19:56:22] Running Scanner on
VM. [ID: 5449648] Executing command via SSH: export
KOKORO_BUILD_NUMBER="11020" export
KOKORO_JOB_NAME="gem5/gcp_ubuntu/presubmit" source
"$HOME/.kokoro_python_vars" ; "$KOKORO_PYTHON_COMMAND"
/tmpfs/log4j_scanner.py Found no vulnerable JARs. [ID: 5449648] Build
finished after 2 secs, exit value: 0
```

I suspect Google has revamped this system at some point to, at least, add
this checker. Perhaps that is what's been causing problems? Perhaps we've
been pushing some patches while someone behind the scenes has been playing
with the server?

Kokoro is acting a bit odd elsewhere. In this chain
https://gem5-review.googlesource.com/c/public/gem5/+/56203, for example,
Kokoro just didn't start at all.

--
Dr. Bobby R. Bruce
Room 3050,
Kemper Hall, UC Davis
Davis,
CA, 95616

web: https://www.bobbybruce.net


On Fri, Feb 4, 2022 at 1:56 AM Giacomo Travaglini <
giacomo.travagl...@arm.com> wrote:
>
> Thanks Bobby,
>
>
>
> Here are some examples:
>
> https://gem5-review.googlesource.com/c/public/gem5/+/56426
>
> https://gem5-review.googlesource.com/c/public/gem5/+/55964/6
>
> https://gem5-review.googlesource.com/c/public/gem5/+/56303/1
>
>
>
> (If you check the verification history of the last patch you can see it
first failed due to timeout. This has become very common in the past days;
failing the first run, rebasing to re-trigger kokoro and hoping next time
It won’t timeout)
>
>
>
> Kind Regards
>
>
>
> Giacomo
>
>
>
> From: Bobby Bruce 
> Date: Thursday, 3 February 2022 at 19:52
> To: Giacomo Travaglini 
> Cc: gem5-dev@gem5.org 
> Subject: Re: Kokoro failures
>
> Are there examples of this timeout happening recently? I can't see any
over the past week.
>
>
>
> There's a separate issue affecting one of Gabe's patches that I'm looking
into (here: https://gem5-review.googlesource.com/c/public/gem5/+/56303) but
these appear to be due to dynamic libraries not linking correctly.
>
>
>
> The only change in testing in the last month has been using clang-11
instead of clang-9 to do the clang compilation check. It's possible
clang-11 takes longer than clang-9 did? In December I increase the timeout
to 7 hours to give us more time to run tests as we were experiencing some
timeouts, I think due to the inclusion of some more tests.
>
>
>
> We could move some tests to the Nightly run fairly easily.
>
>
>
> --
>
> Dr. Bobby R. Bruce
> Room 3050,
> Kemper Hall, UC Davis
> Davis,
> CA, 95616
>
>
>
> web: https://www.bobbybruce.net
>
>
>
>
>
> On Thu, Feb 3, 2022 at 7:00 AM Giacomo Travaglini <
giacomo.travagl...@arm.com> wrote:
>
> Hi all,
>
>
>
> I am seeing a lot of failures on kokoro due to timeouts.
>
> Have we recently added some extra tests to the quick/pre-submit
regressions?
>
>
>
> Kind Regards
>
>
>
> Giacomo
>
> 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.
>
> 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[develop]: arch-x86: Filter out the NMI masking bit from the CMOS offset.

2022-02-04 Thread Gabe Black (Gerrit) via gem5-dev 
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/55244 )


Change subject: arch-x86: Filter out the NMI masking bit from the CMOS  
offset.

..

arch-x86: Filter out the NMI masking bit from the CMOS offset.

A (another) weird/terrible quirk of the architecture of the PC is that
the the highest order bit of the value which selects a register to read
from the CMOS NVRAM/RTC is stolen and repurposed to enable/disable NMIs
since the 286, if the internet is to be believed.

Fortunately We don't currently attempt to generate an NMI anywhere, and so
this bit won't do anything in gem5 currently.

Unfortunately if we treat this value as the real offset without masking
off this bit, if software attempts to disable NMIs with it, it will
trigger an out of bounds assert in the CMOS device.

This change makes the CMOS device slightly smarter and has it maintain
but ignore the NMI disabling bit.

Change-Id: I8d7d0f1b0aadcca2060bf6a27931035859d66ca7
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/55244
Reviewed-by: Gabe Black 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/dev/x86/cmos.cc
M src/dev/x86/cmos.hh
2 files changed, 38 insertions(+), 3 deletions(-)

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




diff --git a/src/dev/x86/cmos.cc b/src/dev/x86/cmos.cc
index 19fab68..d141922 100644
--- a/src/dev/x86/cmos.cc
+++ b/src/dev/x86/cmos.cc
@@ -56,7 +56,7 @@
 pkt->setLE(address);
 break;
   case 0x1:
-pkt->setLE(readRegister(address));
+pkt->setLE(readRegister(address.regNum));
 break;
   default:
 panic("Read from undefined CMOS port.\n");
@@ -75,7 +75,8 @@
 address = pkt->getLE();
 break;
   case 0x1:
-writeRegister(address, pkt->getLE());
+// Ignore the NMI mask bit since we never try to generate one  
anyway.

+writeRegister(address.regNum, pkt->getLE());
 break;
   default:
 panic("Write to undefined CMOS port.\n");
diff --git a/src/dev/x86/cmos.hh b/src/dev/x86/cmos.hh
index 72937af..de64d5a 100644
--- a/src/dev/x86/cmos.hh
+++ b/src/dev/x86/cmos.hh
@@ -29,6 +29,7 @@
 #ifndef __DEV_X86_CMOS_HH__
 #define __DEV_X86_CMOS_HH__

+#include "base/bitunion.hh"
 #include "dev/intpin.hh"
 #include "dev/io_device.hh"
 #include "dev/mc146818.hh"
@@ -45,7 +46,12 @@
   protected:
 Tick latency;

-uint8_t address;
+BitUnion8(CmosAddress)
+Bitfield<6, 0> regNum;
+Bitfield<7> nmiMask;
+EndBitUnion(CmosAddress)
+
+CmosAddress address;

 static const int numRegs = 128;


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/55244
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: I8d7d0f1b0aadcca2060bf6a27931035859d66ca7
Gerrit-Change-Number: 55244
Gerrit-PatchSet: 11
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bradford Beckmann 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Matt Sinclair 
Gerrit-Reviewer: Matthew Poremba 
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]: python: Remove the m5.config and options.py mechanism.

2022-02-04 Thread Gabe Black (Gerrit) via gem5-dev 
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56387 )


Change subject: python: Remove the m5.config and options.py mechanism.
..

python: Remove the m5.config and options.py mechanism.

It appears that there is a mechanism where you could either have a .m5
directory in your home directory, or set an M5_CONFIG environment
variable to some other directory, where you could put an options.py
file. That file would then be passed the options dict which gem5's main
had extracted from its args, which it could modify as it liked.

First, I suspect that this mechanism was basically unknown and was just
a dark corner of gem5 people had forgotten about. Getting rid of it
will help clear out old cruft.

Second, this sort of file reaching in and fiddling with gem5's internal
data structures is dangerous and fragile, and could in almost any case
be replaced with a wrapper script or shell alias.

Change-Id: Ic828716979ea6379f60de796d23281ab075b38ec
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56387
Reviewed-by: Gabe Black 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/python/SConscript
D src/python/m5/config.py
M src/python/m5/main.py
3 files changed, 27 insertions(+), 57 deletions(-)

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




diff --git a/src/python/SConscript b/src/python/SConscript
index 984ae82..343a696 100644
--- a/src/python/SConscript
+++ b/src/python/SConscript
@@ -217,7 +217,6 @@
 PySource('', 'importer.py')
 PySource('m5', 'm5/__init__.py')
 PySource('m5', 'm5/SimObject.py')
-PySource('m5', 'm5/config.py')
 PySource('m5', 'm5/core.py')
 PySource('m5', 'm5/debug.py')
 PySource('m5', 'm5/event.py')
diff --git a/src/python/m5/config.py b/src/python/m5/config.py
deleted file mode 100644
index 926ea14..000
--- a/src/python/m5/config.py
+++ /dev/null
@@ -1,48 +0,0 @@
-# Copyright (c) 2008 The Hewlett-Packard Development Company
-# All rights reserved.
-#
-# 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.
-
-import os
-from os.path import isdir, isfile, join as joinpath
-
-
-confdir = os.environ.get('M5_CONFIG')
-
-if not confdir:
-# HOME is not set when running regressions, due to use of scons
-# Execute() function.
-homedir = os.environ.get('HOME')
-if homedir and isdir(joinpath(homedir, '.m5')):
-confdir = joinpath(homedir, '.m5')
-
-def get(name):
-if not confdir:
-return None
-conffile = joinpath(confdir, name)
-if not isfile(conffile):
-return None
-
-return conffile
-
diff --git a/src/python/m5/main.py b/src/python/m5/main.py
index f649e77..701d9f6 100644
--- a/src/python/m5/main.py
+++ b/src/python/m5/main.py
@@ -61,7 +61,6 @@


 def parse_options():
-from . import config
 from .options import OptionParser

 options = OptionParser(usage=usage, description=brief_copyright)
@@ -155,13 +154,6 @@
 option("--list-sim-objects", action='store_true', default=False,
 help="List all built-in SimObjects, their params and default  
values")


-# load the options.py config file to allow people to set their own
-# default options
-options_file = config.get('options.py')
-if options_file:
-scope = { 'options' : options }
-exec(compile(open(options_file).read(), options_file, 'exec'),  
scope)

-
 arguments = options.parse_args()
 return options,arguments


--
To view, visit https://gem5-revi

[gem5-dev] Change in gem5/gem5[develop]: scons: Make CC, CXX and PROTOC no longer sticky.

2022-02-04 Thread Gabe Black (Gerrit) via gem5-dev 
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56386 )


Change subject: scons: Make CC, CXX and PROTOC no longer sticky.
..

scons: Make CC, CXX and PROTOC no longer sticky.

These variables will always be imported from the environment, and never
from previous builds. For SCons to actually use these values, they need
to not only be in the environment variables external commands SCons runs
sees, they also have to be promoted to actual SCons construction
variables.

Change-Id: I54035442d70972396f1788bd0773877222d7a7c5
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56386
Reviewed-by: Andreas Sandberg 
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M SConstruct
M site_scons/gem5_scons/defaults.py
2 files changed, 30 insertions(+), 3 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
  Andreas Sandberg: Looks good to me, but someone else must approve
  kokoro: Regressions pass




diff --git a/SConstruct b/SConstruct
index d7cd564..6cdffe5 100755
--- a/SConstruct
+++ b/SConstruct
@@ -253,8 +253,6 @@
 global_vars = Variables(global_vars_file, args=ARGUMENTS)

 global_vars.AddVariables(
-('CC', 'C compiler', environ.get('CC', main['CC'])),
-('CXX', 'C++ compiler', environ.get('CXX', main['CXX'])),
 ('CCFLAGS_EXTRA', 'Extra C and C++ compiler flags', ''),
 ('GEM5PY_CCFLAGS_EXTRA', 'Extra C and C++ gem5py compiler flags', ''),
 ('GEM5PY_LINKFLAGS_EXTRA', 'Extra marshal gem5py flags', ''),
@@ -262,7 +260,6 @@
 ('PYTHON_CONFIG', 'Python config binary to use',
  [ 'python3-config', 'python-config']
 ),
-('PROTOC', 'protoc tool', environ.get('PROTOC', 'protoc')),
 ('BATCH', 'Use batch pool for build and tests', False),
 ('BATCH_CMD', 'Batch pool submission command name', 'qdo'),
 ('M5_BUILD_CACHE', 'Cache built objects in this directory', False),
diff --git a/site_scons/gem5_scons/defaults.py  
b/site_scons/gem5_scons/defaults.py

index 1f8d5d5..4efaa26 100644
--- a/site_scons/gem5_scons/defaults.py
+++ b/site_scons/gem5_scons/defaults.py
@@ -63,6 +63,16 @@
 any([key.startswith(prefix) for prefix in use_prefixes]):
 env['ENV'][key] = val

+# These variables from the environment override/become SCons variables,
+# with a default if they weren't in the host environment.
+var_overrides = {
+'CC': env['CC'],
+'CXX': env['CXX'],
+'PROTOC': 'protoc'
+}
+for key,default in var_overrides.items():
+env[key] = env['ENV'].get(key, default)
+
 # Tell scons to avoid implicit command dependencies to avoid issues
 # with the param wrappes being compiled twice (see
 # https://github.com/SCons/scons/issues/2811

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/56386
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: I54035442d70972396f1788bd0773877222d7a7c5
Gerrit-Change-Number: 56386
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Jason Lowe-Power 
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-x86: Use operand size consistently pushing for near calls.

2022-02-04 Thread Gabe Black (Gerrit) via gem5-dev 
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/55250 )


Change subject: arch-x86: Use operand size consistently pushing for near  
calls.

..

arch-x86: Use operand size consistently pushing for near calls.

The near call instruction first attempts to store the return address on
the stack (the part of the instruction that could fail), and then if
that succeeds it decrements the stack pointer to point at the newly
stored data.

Unfortunately, the microcode was not using the same offset between
those two steps. Specifically it was using the effective operand size
when storing the return address, but then incorrectly using the
effective stack size when adjusting the stack pointer.

This change updates the microcode to use the effective operand size in
both places.

Change-Id: Ic4211a96900fee5d10c2fa0e038070383fefaac3
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/55250
Maintainer: Bobby Bruce 
Tested-by: kokoro 
Reviewed-by: Gabe Black 
---
M src/arch/x86/isa/insts/general_purpose/control_transfer/call.py
1 file changed, 30 insertions(+), 4 deletions(-)

Approvals:
  Gabe Black: Looks good to me, approved
  Bobby Bruce: Looks good to me, approved
  kokoro: Regressions pass




diff --git  
a/src/arch/x86/isa/insts/general_purpose/control_transfer/call.py  
b/src/arch/x86/isa/insts/general_purpose/control_transfer/call.py

index 4204a8c..31b81e9 100644
--- a/src/arch/x86/isa/insts/general_purpose/control_transfer/call.py
+++ b/src/arch/x86/isa/insts/general_purpose/control_transfer/call.py
@@ -45,7 +45,7 @@
 rdip t7
 # Check target of call
 stis t7, ss, [0, t0, rsp], "-env.dataSize"
-subi rsp, rsp, ssz
+subi rsp, rsp, dsz
 wrip t7, t1
 };

@@ -59,7 +59,7 @@
 rdip t1
 # Check target of call
 stis t1, ss, [0, t0, rsp], "-env.dataSize"
-subi rsp, rsp, ssz
+subi rsp, rsp, dsz
 wripi reg, 0
 };

@@ -74,7 +74,7 @@
 ld t1, seg, sib, disp
 # Check target of call
 st t7, ss, [0, t0, rsp], "-env.dataSize"
-subi rsp, rsp, ssz
+subi rsp, rsp, dsz
 wripi t1, 0
 };

@@ -89,7 +89,7 @@
 ld t1, seg, riprel, disp
 # Check target of call
 st t7, ss, [0, t0, rsp], "-env.dataSize"
-subi rsp, rsp, ssz
+subi rsp, rsp, dsz
 wripi t1, 0
 };
 '''

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/55250
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: Ic4211a96900fee5d10c2fa0e038070383fefaac3
Gerrit-Change-Number: 55250
Gerrit-PatchSet: 12
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby Bruce 
Gerrit-Reviewer: Bradford Beckmann 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Matt Sinclair 
Gerrit-Reviewer: Matthew Poremba 
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,arch-arm: Track register size in RegClassInfo.

2022-02-04 Thread Gabe Black (Gerrit) via gem5-dev 
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/49104 )


 (

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

 )Change subject: cpu,arch-arm: Track register size in RegClassInfo.
..

cpu,arch-arm: Track register size in RegClassInfo.

By default, registers are the size of RegVal, the type often used to
store them. For some types of registers, like vector or vector predicate
registers, the size of each individual register is larger, and can't fit
in a primitive type.

To help facilitate storing even these outliers in a generalized way,
this change adds two fields to RegClassInfo to track the size of
individual registers. One tracks the raw size of the registers
themselves, and the other tracks the minimal shift necessary to find the
offset of a register in a contiguous(ish) array of bytes. By forcing
each register to be aligned to a power of two boundary, we avoid having
to do a multiplication to find their address even if the registers are
oddly sized. We can instead do a shift with a precomputed shift amount
which should be faster.

Change-Id: I035f1b4cb00ece4e8306d7953ea358af75a0d1de
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49104
Reviewed-by: Giacomo Travaglini 
Maintainer: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/arch/arm/isa.cc
M src/cpu/reg_class.hh
2 files changed, 45 insertions(+), 8 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 5c8c743..3210351 100644
--- a/src/arch/arm/isa.cc
+++ b/src/arch/arm/isa.cc
@@ -88,10 +88,10 @@
 {
 _regClasses.emplace_back(NUM_INTREGS, INTREG_ZERO);
 _regClasses.emplace_back(0);
-_regClasses.emplace_back(NumVecRegs);
+_regClasses.emplace_back(NumVecRegs, -1, sizeof(VecRegContainer));
 _regClasses.emplace_back(NumVecRegs * NumVecElemPerVecReg,
 vecRegElemClassOps);
-_regClasses.emplace_back(NumVecPredRegs);
+_regClasses.emplace_back(NumVecPredRegs, -1,  
sizeof(VecPredRegContainer));

 _regClasses.emplace_back(NUM_CCREGS);
 _regClasses.emplace_back(NUM_MISCREGS, miscRegClassOps);

diff --git a/src/cpu/reg_class.hh b/src/cpu/reg_class.hh
index ab3e8f9..5664bc3 100644
--- a/src/cpu/reg_class.hh
+++ b/src/cpu/reg_class.hh
@@ -41,12 +41,11 @@
 #ifndef __CPU__REG_CLASS_HH__
 #define __CPU__REG_CLASS_HH__

-#include 
 #include 
 #include 

+#include "base/intmath.hh"
 #include "base/types.hh"
-#include "config/the_isa.hh"

 namespace gem5
 {
@@ -97,22 +96,32 @@
   private:
 size_t _numRegs;
 const RegIndex _zeroReg;
+size_t _regBytes;
+// This is how much to shift an index by to get an offset of a  
register in
+// a register file from the register index, which would otherwise need  
to

+// be calculated with a multiply.
+size_t _regShift;

 static inline DefaultRegClassOps defaultOps;
 RegClassOps *_ops = &defaultOps;

   public:
-RegClass(size_t num_regs, RegIndex new_zero=-1) :
-_numRegs(num_regs), _zeroReg(new_zero)
+RegClass(size_t num_regs, RegIndex new_zero=-1,
+size_t reg_bytes=sizeof(RegVal)) :
+_numRegs(num_regs), _zeroReg(new_zero), _regBytes(reg_bytes),
+_regShift(ceilLog2(reg_bytes))
 {}
-RegClass(size_t num_regs, RegClassOps &new_ops, RegIndex new_zero=-1) :
-RegClass(num_regs, new_zero)
+RegClass(size_t num_regs, RegClassOps &new_ops, RegIndex new_zero=-1,
+size_t reg_bytes=sizeof(RegVal)) :
+RegClass(num_regs, new_zero, reg_bytes)
 {
 _ops = &new_ops;
 }

 size_t numRegs() const { return _numRegs; }
 RegIndex zeroReg() const { return _zeroReg; }
+size_t regBytes() const { return _regBytes; }
+size_t regShift() const { return _regShift; }

 std::string regName(const RegId &id) const { return _ops->regName(id);  
}

 };

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49104
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: I035f1b4cb00ece4e8306d7953ea358af75a0d1de
Gerrit-Change-Number: 49104
Gerrit-PatchSet: 40
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-CC: Jason Lowe-Power 
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: rename RegClass::size to RegClass::numRegs.

2022-02-04 Thread Gabe Black (Gerrit) via gem5-dev 
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56303 )


Change subject: cpu: rename RegClass::size to RegClass::numRegs.
..

cpu: rename RegClass::size to RegClass::numRegs.

This will make the coming addition of a regBytes method less ambiguous.

Change-Id: If4b9369dbe484154eec7bf651642cb1d820283e4
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56303
Reviewed-by: Giacomo Travaglini 
Maintainer: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/cpu/minor/scoreboard.hh
M src/cpu/o3/cpu.cc
M src/cpu/o3/inst_queue.cc
M src/cpu/o3/regfile.cc
M src/cpu/o3/rename_map.cc
M src/cpu/reg_class.hh
M src/cpu/simple_thread.cc
M src/cpu/thread_context.cc
8 files changed, 75 insertions(+), 57 deletions(-)

Approvals:
  Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/cpu/minor/scoreboard.hh b/src/cpu/minor/scoreboard.hh
index 3ae0b65..d091b44 100644
--- a/src/cpu/minor/scoreboard.hh
+++ b/src/cpu/minor/scoreboard.hh
@@ -114,11 +114,12 @@
 Named(name),
 regClasses(reg_classes),
 intRegOffset(0),
-floatRegOffset(intRegOffset + reg_classes.at(IntRegClass).size()),
-ccRegOffset(floatRegOffset + reg_classes.at(FloatRegClass).size()),
-vecRegOffset(ccRegOffset + reg_classes.at(CCRegClass).size()),
-vecPredRegOffset(vecRegOffset +  
reg_classes.at(VecElemClass).size()),

-numRegs(vecPredRegOffset + reg_classes.at(VecPredRegClass).size()),
+floatRegOffset(intRegOffset +  
reg_classes.at(IntRegClass).numRegs()),
+ccRegOffset(floatRegOffset +  
reg_classes.at(FloatRegClass).numRegs()),

+vecRegOffset(ccRegOffset + reg_classes.at(CCRegClass).numRegs()),
+vecPredRegOffset(vecRegOffset +
+reg_classes.at(VecElemClass).numRegs()),
+numRegs(vecPredRegOffset +  
reg_classes.at(VecPredRegClass).numRegs()),

 zeroReg(reg_classes.at(IntRegClass).zeroReg()),
 numResults(numRegs, 0),
 numUnpredictableResults(numRegs, 0),
diff --git a/src/cpu/o3/cpu.cc b/src/cpu/o3/cpu.cc
index 3b4e15f..e05ea92 100644
--- a/src/cpu/o3/cpu.cc
+++ b/src/cpu/o3/cpu.cc
@@ -195,19 +195,20 @@
 const auto ®Classes = params.isa[0]->regClasses();

 assert(params.numPhysIntRegs >=
-numThreads * regClasses.at(IntRegClass).size());
+numThreads * regClasses.at(IntRegClass).numRegs());
 assert(params.numPhysFloatRegs >=
-numThreads * regClasses.at(FloatRegClass).size());
+numThreads * regClasses.at(FloatRegClass).numRegs());
 assert(params.numPhysVecRegs >=
-numThreads * regClasses.at(VecRegClass).size());
+numThreads * regClasses.at(VecRegClass).numRegs());
 assert(params.numPhysVecPredRegs >=
-numThreads * regClasses.at(VecPredRegClass).size());
+numThreads * regClasses.at(VecPredRegClass).numRegs());
 assert(params.numPhysCCRegs >=
-numThreads * regClasses.at(CCRegClass).size());
+numThreads * regClasses.at(CCRegClass).numRegs());

 // Just make this a warning and go ahead anyway, to keep from having to
 // add checks everywhere.
-warn_if(regClasses.at(CCRegClass).size() == 0 &&  
params.numPhysCCRegs != 0,

+warn_if(regClasses.at(CCRegClass).numRegs() == 0 &&
+params.numPhysCCRegs != 0,
 "Non-zero number of physical CC regs specified, even though\n"
 "ISA does not use them.");

@@ -224,7 +225,7 @@
 // Initialize rename map to assign physical registers to the
 // architectural registers for active threads only.
 for (ThreadID tid = 0; tid < active_threads; tid++) {
-for (RegIndex ridx = 0; ridx < regClasses.at(IntRegClass).size();
+for (RegIndex ridx = 0; ridx <  
regClasses.at(IntRegClass).numRegs();

 ++ridx) {
 // Note that we can't use the rename() method because we don't
 // want special treatment for the zero register at this point
@@ -233,7 +234,7 @@
 commitRenameMap[tid].setEntry(RegId(IntRegClass, ridx),  
phys_reg);

 }

-for (RegIndex ridx = 0; ridx < regClasses.at(FloatRegClass).size();
+for (RegIndex ridx = 0; ridx <  
regClasses.at(FloatRegClass).numRegs();

 ++ridx) {
 PhysRegIdPtr phys_reg = freeList.getFloatReg();
 renameMap[tid].setEntry(RegId(FloatRegClass, ridx), phys_reg);
@@ -241,7 +242,7 @@
 RegId(FloatRegClass, ridx), phys_reg);
 }

-const size_t numVecs = regClasses.at(VecRegClass).size();
+const size_t numVecs = regClasses.at(VecRegClass).numRegs();
 /* Initialize the full-vector interface */
 for (RegIndex ridx = 0; ridx < numVecs; ++ridx) {
 RegId rid = RegId(VecRegClass, ridx);
@@ -250,7

[gem5-dev] Change in gem5/gem5[develop]: cpu-kvm,sim: Reverse the relationship between System and KvmVM.

2022-02-04 Thread Gabe Black (Gerrit) via gem5-dev 
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/56187 )


 (

2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.
 )Change subject: cpu-kvm,sim: Reverse the relationship between System and  
KvmVM.

..

cpu-kvm,sim: Reverse the relationship between System and KvmVM.

The KvmVM will declare itself to the System object, instead of the other
way around. This way the System object can just keep an opaque KvmVM
pointer which does not depend on the KvmVM code even being compiled into
gem5. If there is a KvmVM object, that can more safely assume there is a
corresponding System object to attach itself to.

Also move use of the KvmVM pointer out of constructors, since the VM may
not have registered itself with the System object yet. Those uses can
happen in the init() method instead.

Change-Id: Ia0842612b101315bc1af0232d7f5ae2b55a15922
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/56187
Reviewed-by: Giacomo Travaglini 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/arch/arm/kvm/arm_cpu.cc
M src/arch/arm/kvm/base_cpu.cc
M src/arch/x86/kvm/x86_cpu.cc
M src/arch/x86/kvm/x86_cpu.hh
M src/cpu/kvm/KvmVM.py
M src/cpu/kvm/base.cc
M src/cpu/kvm/base.hh
M src/cpu/kvm/vm.cc
M src/cpu/kvm/vm.hh
M src/sim/System.py
M src/sim/system.cc
M src/sim/system.hh
12 files changed, 63 insertions(+), 54 deletions(-)

Approvals:
  Giacomo Travaglini: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/arch/arm/kvm/arm_cpu.cc b/src/arch/arm/kvm/arm_cpu.cc
index ecdc602..33237c9 100644
--- a/src/arch/arm/kvm/arm_cpu.cc
+++ b/src/arch/arm/kvm/arm_cpu.cc
@@ -374,12 +374,12 @@
 if (fiqAsserted != simFIQ) {
 fiqAsserted = simFIQ;
 DPRINTF(KvmInt, "KVM: Update FIQ state: %i\n", simFIQ);
-vm.setIRQLine(interruptVcpuFiq(vcpuID), simFIQ);
+vm->setIRQLine(interruptVcpuFiq(vcpuID), simFIQ);
 }
 if (irqAsserted != simIRQ) {
 irqAsserted = simIRQ;
 DPRINTF(KvmInt, "KVM: Update IRQ state: %i\n", simIRQ);
-vm.setIRQLine(interruptVcpuIrq(vcpuID), simIRQ);
+vm->setIRQLine(interruptVcpuIrq(vcpuID), simIRQ);
 }

 return BaseKvmCPU::kvmRun(ticks);
diff --git a/src/arch/arm/kvm/base_cpu.cc b/src/arch/arm/kvm/base_cpu.cc
index fe922a1..f18d697 100644
--- a/src/arch/arm/kvm/base_cpu.cc
+++ b/src/arch/arm/kvm/base_cpu.cc
@@ -102,13 +102,13 @@
 struct kvm_vcpu_init target_config;
 memset(&target_config, 0, sizeof(target_config));

-vm.kvmArmPreferredTarget(target_config);
+vm->kvmArmPreferredTarget(target_config);
 if (!((ArmSystem *)system)->highestELIs64()) {
 target_config.features[0] |= (1 << KVM_ARM_VCPU_EL1_32BIT);
 }
 kvmArmVCpuInit(target_config);

-if (!vm.hasKernelIRQChip())
+if (!vm->hasKernelIRQChip())
 virtTimerPin = static_cast(system)\
 ->getGenericTimer()->params().int_virt->get(tc);
 }
@@ -120,14 +120,14 @@
 const bool simFIQ(interrupt->checkRaw(INT_FIQ));
 const bool simIRQ(interrupt->checkRaw(INT_IRQ));

-if (!vm.hasKernelIRQChip()) {
+if (!vm->hasKernelIRQChip()) {
 if (fiqAsserted != simFIQ) {
 DPRINTF(KvmInt, "KVM: Update FIQ state: %i\n", simFIQ);
-vm.setIRQLine(INTERRUPT_VCPU_FIQ(vcpuID), simFIQ);
+vm->setIRQLine(INTERRUPT_VCPU_FIQ(vcpuID), simFIQ);
 }
 if (irqAsserted != simIRQ) {
 DPRINTF(KvmInt, "KVM: Update IRQ state: %i\n", simIRQ);
-vm.setIRQLine(INTERRUPT_VCPU_IRQ(vcpuID), simIRQ);
+vm->setIRQLine(INTERRUPT_VCPU_IRQ(vcpuID), simIRQ);
 }
 } else {
 warn_if(simFIQ && !fiqAsserted,
@@ -144,7 +144,7 @@

 Tick kvmRunTicks = BaseKvmCPU::kvmRun(ticks);

-if (!vm.hasKernelIRQChip()) {
+if (!vm->hasKernelIRQChip()) {
 uint64_t device_irq_level =
 getKvmRunState()->s.regs.device_irq_level;

diff --git a/src/arch/x86/kvm/x86_cpu.cc b/src/arch/x86/kvm/x86_cpu.cc
index 34ac704..af041a5 100644
--- a/src/arch/x86/kvm/x86_cpu.cc
+++ b/src/arch/x86/kvm/x86_cpu.cc
@@ -536,8 +536,14 @@
 X86KvmCPU::X86KvmCPU(const X86KvmCPUParams ¶ms)
 : BaseKvmCPU(params),
   useXSave(params.useXSave)
+{}
+
+void
+X86KvmCPU::init()
 {
-Kvm &kvm(*vm.kvm);
+BaseKvmCPU::init();
+
+Kvm &kvm = *vm->kvm;

 if (!kvm.capSetTSSAddress())
 panic("KVM: Missing capability (KVM_CAP_SET_TSS_ADDR)\n");
@@ -667,7 +673,7 @@
 void
 X86KvmCPU::dumpMSRs() const
 {
-const Kvm::MSRIndexVector &supported_msrs(vm.kvm->getSupportedMSRs());
+const Kvm::MSRIndexVector &supported_msrs =  
vm->kvm->getSupportedMSRs();

 auto msrs = newVarStruct(
 supported_msrs.size());

@@ -1549,7 +1555,7 @@
 X86KvmCPU::getMsrIntersection() const
 {
 if (cachedMsrIntersection.emp

[gem5-dev] Re: Kokoro failures

2022-02-04 Thread Giacomo Travaglini via gem5-dev 
Thanks Bobby,

Here are some examples:
https://gem5-review.googlesource.com/c/public/gem5/+/56426
https://gem5-review.googlesource.com/c/public/gem5/+/55964/6
https://gem5-review.googlesource.com/c/public/gem5/+/56303/1

(If you check the verification history of the last patch you can see it first 
failed due to timeout. This has become very common in the past days; failing 
the first run, rebasing to re-trigger kokoro and hoping next time It won’t 
timeout)

Kind Regards

Giacomo

From: Bobby Bruce 
Date: Thursday, 3 February 2022 at 19:52
To: Giacomo Travaglini 
Cc: gem5-dev@gem5.org 
Subject: Re: Kokoro failures
Are there examples of this timeout happening recently? I can't see any over the 
past week.

There's a separate issue affecting one of Gabe's patches that I'm looking into 
(here: https://gem5-review.googlesource.com/c/public/gem5/+/56303) but these 
appear to be due to dynamic libraries not linking correctly.

The only change in testing in the last month has been using clang-11 instead of 
clang-9 to do the clang compilation check. It's possible clang-11 takes longer 
than clang-9 did? In December I increase the timeout to 7 hours to give us more 
time to run tests as we were experiencing some timeouts, I think due to the 
inclusion of some more tests.

We could move some tests to the Nightly run fairly easily.

--
Dr. Bobby R. Bruce
Room 3050,
Kemper Hall, UC Davis
Davis,
CA, 95616


web: https://www.bobbybruce.net



On Thu, Feb 3, 2022 at 7:00 AM Giacomo Travaglini 
mailto:giacomo.travagl...@arm.com>> wrote:
Hi all,

I am seeing a lot of failures on kokoro due to timeouts.
Have we recently added some extra tests to the quick/pre-submit regressions?

Kind Regards

Giacomo
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.
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