[gem5-dev] Change in gem5/gem5[develop]: scons: Enable LTO for opt builds.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40815 )



Change subject: scons: Enable LTO for opt builds.
..

scons: Enable LTO for opt builds.

The name of the build is opt, so it should be fully optomized. Also, the
fast build, the only one with LTO historically, is dangerous to use
since it disables many error checks. I personally run gem5 many times
while developing, iterating and trying to fix bugs, and so want it to
run quickly then too, not just the final time when collecting results.

This has the nice side effect of speeding up the build time of build/X86
significantly (6:20 -> 4:27) due to parallelization of the link, and reduces
the size of the build/X86 directory (with debug compression enabled) from
3.4GB to 2.8GB.

The size of build/X86/python/_m5 is still 1.6GB, so still more than half
of the total size of build/X86.

The debug section compression flags were changed from -Wl,... and
-Wa,... to -gz because the LTO wrapper complains about -Wa options. The
-gz option seems to get the same result, but goes through the compiler
driver without tunneling through it to the assembler or linker with -Wa
or -Wl.

Change-Id: I8feabf99454693fdd100d9e1a64fdeae53362f75
---
M SConstruct
M src/SConscript
2 files changed, 7 insertions(+), 9 deletions(-)



diff --git a/SConstruct b/SConstruct
index b88b150..14c280e 100755
--- a/SConstruct
+++ b/SConstruct
@@ -621,15 +621,12 @@
 CacheDir(main['M5_BUILD_CACHE'])

 if GetOption('compress_debug'):
-AsCompressDebugFlag = '-Wa,--compress-debug-sections=zlib'
-LdCompressDebugFlag = '-Wl,--compress-debug-sections=zlib'
-if conf.CheckCxxFlag(AsCompressDebugFlag):
-main.Append(CXXFLAGS=[AsCompressDebugFlag])
+if conf.CheckCxxFlag('-gz'):
+main.Append(CXXFLAGS=['-gz'])
 else:
 warning("Can't enable object file debug section compression")
-if conf.CheckLinkFlag(LdCompressDebugFlag):
-main.Append(LINKFLAGS=[LdCompressDebugFlag],
-SHLINKFLAGS=[LdCompressDebugFlag])
+if conf.CheckLinkFlag('-gz'):
+main.Append(LINKFLAGS=['-gz'], SHLINKFLAGS=['-gz'])
 else:
 warning("Can't enable executable debug section compression")

diff --git a/src/SConscript b/src/SConscript
index aeb7038..49e9558 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -1344,8 +1344,9 @@
 ccflags[target] += ['-O3']
 ldflags[target] += ['-O3']

-ccflags['fast'] += env['LTO_CCFLAGS']
-ldflags['fast'] += env['LTO_LDFLAGS']
+for target in ['opt', 'fast']:
+ccflags[target] += env['LTO_CCFLAGS']
+ldflags[target] += env['LTO_LDFLAGS']
 elif env['CLANG']:
 ccflags['debug'] += ['-g', '-O0']
 # opt, fast, prof and perf all share the same cc flags

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40815
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: I8feabf99454693fdd100d9e1a64fdeae53362f75
Gerrit-Change-Number: 40815
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]: scons: Remove partial linking.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40795 )



Change subject: scons: Remove partial linking.
..

scons: Remove partial linking.

This feature didn't actually provide any benefit in the end, and
increased build directory size and scons complexity.

Change-Id: Ia5aa16a8dd008599645076cea8131799f6086e0f
---
M SConstruct
M src/SConscript
2 files changed, 4 insertions(+), 141 deletions(-)



diff --git a/SConstruct b/SConstruct
index 8744501..2d158ec 100755
--- a/SConstruct
+++ b/SConstruct
@@ -112,9 +112,6 @@
   help='Compress debug info in build files')
 AddOption('--no-lto', action='store_true',
   help='Disable Link-Time Optimization for fast')
-AddOption('--force-lto', action='store_true',
-  help='Use Link-Time Optimization instead of partial linking' +
-   ' when the compiler doesn\'t support using them together.')
 AddOption('--verbose', action='store_true',
   help='Print full tool command lines')
 AddOption('--without-python', action='store_true',
@@ -130,9 +127,6 @@

 from gem5_scons import Transform, error, warning, summarize_warnings

-if GetOption('no_lto') and GetOption('force_lto'):
-error('--no-lto and --force-lto are mutually exclusive')
-
 
 #
 # Set up the main build environment.
@@ -328,14 +322,8 @@
 # option --as-needed
 if sys.platform != "darwin":
 main.Append(LINKFLAGS='-Wl,--as-needed')
-main['FILTER_PSHLINKFLAGS'] = lambda x: str(x).replace(' -shared', '')
-main['PSHLINKFLAGS'] =  
main.subst('${FILTER_PSHLINKFLAGS(SHLINKFLAGS)}')

 if GetOption('gold_linker'):
 main.Append(LINKFLAGS='-fuse-ld=gold')
-main['PLINKFLAGS'] = main.get('LINKFLAGS')
-shared_partial_flags = ['-r', '-nostdlib']
-main.Append(PSHLINKFLAGS=shared_partial_flags)
-main.Append(PLINKFLAGS=shared_partial_flags)

 # Treat warnings as errors but white list some warnings that we
 # want to allow (e.g., deprecation warnings).
@@ -366,41 +354,10 @@

 main['GCC_VERSION'] = gcc_version

-# Incremental linking with LTO is currently broken in gcc versions
-# 4.9 and above. A version where everything works completely hasn't
-# yet been identified.
-#
-# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67548
-main['BROKEN_INCREMENTAL_LTO'] = True
-
-if compareVersions(gcc_version, '6.0') >= 0:
-# gcc versions 6.0 and greater accept an -flinker-output flag which
-# selects what type of output the linker should generate. This is
-# necessary for incremental lto to work, but is also broken in
-# current versions of gcc. It may not be necessary in future
-# versions. We add it here since it might be, and as a reminder  
that

-# it exists. It's excluded if lto is being forced.
-#
-# https://gcc.gnu.org/gcc-6/changes.html
-# https://gcc.gnu.org/ml/gcc-patches/2015-11/msg03161.html
-# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69866
-if not GetOption('force_lto'):
-main.Append(PSHLINKFLAGS=['-flinker-output=rel'])
-main.Append(PLINKFLAGS=['-flinker-output=rel'])
-
-disable_lto = GetOption('no_lto')
-if not disable_lto and main.get('BROKEN_INCREMENTAL_LTO', False) and \
-not GetOption('force_lto'):
-warning('Your compiler doesn\'t support incremental linking and  
lto '
-'at the same time, so lto is being disabled. To force lto  
on '

-'anyway, use the --force-lto option. That will disable '
-'partial linking.')
-disable_lto = True
-
 # Add the appropriate Link-Time Optimization (LTO) flags
 # unless LTO is explicitly turned off. Note that these flags
 # are only used by the fast target.
-if not disable_lto:
+if not GetOption('no_lto'):
 # Pass the LTO flag when compiling to produce GIMPLE
 # output, we merely create the flags here and only append
 # them later
@@ -1093,32 +1050,6 @@

 main.Append(BUILDERS = { 'ConfigFile' : config_builder })

-###
-#
-# Builders for static and shared partially linked object files.
-#
-###
-
-partial_static_builder = Builder(action=SCons.Defaults.LinkAction,
- src_suffix='$OBJSUFFIX',
- src_builder=['StaticObject', 'Object'],
- LINKFLAGS='$PLINKFLAGS',
- LIBS='')
-
-def partial_shared_emitter(target, source, env):
-for tgt in target:
-tgt.attributes.shared = 1
-return (target, source)
-partial_shared_builder = Builder(action=SCons.Defaults.ShLinkAction,
- 

[gem5-dev] Change in gem5/gem5[develop]: tests,base: Delete the SymbolTable::load method and symtest test.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40620 )


Change subject: tests,base: Delete the SymbolTable::load method and symtest  
test.

..

tests,base: Delete the SymbolTable::load method and symtest test.

This test expects to load a symbol file using the load method of gem5's
SymbolTable class, and then to search through it for a given symbol or
address.

Unfortunately, the type of file it expects to load has a format where
each line is of the form:

0x, symbol_name

where the numerical part is the address of the symbol, and the part
after the comma is the symbol name. I have not been able to find any
tool which outputs a symbol file in this format, or any tool for
inspecting an existing object file which will output symbols in this
format. I looked at objdump, objcopy, nm, and the map file format output
by gnu's linker. nm has 3 different output formats, none of which match.
Usually when working with ELF files, one would just generate a new ELF
file which only had debugging information like the symbol table, and
then strip the symbols out of the original.

Since this file format seems to have been invented from thin air, there
isn't really a good way to generate a canonical file to test the loading
code against, nor is being able to load this obscure format likely to be
useful to anybody. If someone *did* want to load an external symbol
table, they would use the ELF loader and not this.

This CL deletes both this test, and the loading code in SymbolTable.

Change-Id: I20402e3f35e54d1e186a92d9c83d1c06ec86bf7d
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40620
Reviewed-by: Daniel Carvalho 
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
---
M src/base/loader/symtab.cc
M src/base/loader/symtab.hh
M src/unittest/SConscript
D src/unittest/symtest.cc
4 files changed, 0 insertions(+), 125 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, approved
  Bobby R. Bruce: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/base/loader/symtab.cc b/src/base/loader/symtab.cc
index c2c53cc..0d0e826 100644
--- a/src/base/loader/symtab.cc
+++ b/src/base/loader/symtab.cc
@@ -85,46 +85,6 @@
 return true;
 }

-bool
-SymbolTable::load(const std::string )
-{
-std::string buffer;
-std::ifstream file(filename.c_str());
-
-if (!file)
-fatal("file error: Can't open symbol table file %s\n", filename);
-
-while (!file.eof()) {
-getline(file, buffer);
-if (buffer.empty())
-continue;
-
-std::string::size_type idx = buffer.find(',');
-if (idx == std::string::npos)
-return false;
-
-std::string address = buffer.substr(0, idx);
-eat_white(address);
-if (address.empty())
-return false;
-
-std::string name = buffer.substr(idx + 1);
-eat_white(name);
-if (name.empty())
-return false;
-
-Addr addr;
-if (!to_number(address, addr))
-return false;
-
-if (!insert({ Symbol::Binding::Global, name, addr }))
-return false;
-}
-
-file.close();
-return true;
-}
-
 void
 SymbolTable::serialize(const std::string , CheckpointOut ) const
 {
diff --git a/src/base/loader/symtab.hh b/src/base/loader/symtab.hh
index a0203a6..5610544 100644
--- a/src/base/loader/symtab.hh
+++ b/src/base/loader/symtab.hh
@@ -129,7 +129,6 @@
 // into this one.
 bool insert(const Symbol );
 bool insert(const SymbolTable );
-bool load(const std::string );
 bool empty() const { return symbols.empty(); }

 SymbolTablePtr
diff --git a/src/unittest/SConscript b/src/unittest/SConscript
index 9ebe863..5008066 100644
--- a/src/unittest/SConscript
+++ b/src/unittest/SConscript
@@ -32,5 +32,3 @@

 stattest_py = PySource('m5', 'stattestmain.py', tags='stattest')
 UnitTest('stattest', 'stattest.cc', with_tag('stattest'), main=True)
-
-UnitTest('symtest', 'symtest.cc')
diff --git a/src/unittest/symtest.cc b/src/unittest/symtest.cc
deleted file mode 100644
index 6de3c8d..000
--- a/src/unittest/symtest.cc
+++ /dev/null
@@ -1,82 +0,0 @@
-/*
- * Copyright (c) 2002-2005 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 

[gem5-dev] Change in gem5/gem5[develop]: arch-power: Restore consistency with other platforms

[Boris Shingarov (Gerrit) via gem5-dev]

Boris Shingarov has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40635 )


Change subject: arch-power: Restore consistency with other platforms
..

arch-power: Restore consistency with other platforms

The 32-bit POWER reference test binary was removed in c1ebdf66f
(as a nasty surprise for POWER users).

The remaining platforms split between two approaches:

MIPS rebuilds "hello" from source.
This fails for two reasons:
1) The trivial reason is that on POWER make abends due to no makefile.
2) The more fundamental reason is that gem5 is not completely bug-free
(especially the Decoder on POWER in this case), therefore regression
testing is only possible if we have not just some hello program, but
a very particular bit sequence to serve as an immutable reference.

ARM and X86 follow the reference-bit-sequence approach.  POWER will
be consistent with same.  Including the sha1 for hello32,
77b27b67393311546e768b5ff35202490bad71aa, as a simple immutability
assurance.  I have also renamed hello to hello32 in anticipation to
merge Sandipan's e52dbcb.

Change-Id: I77ef31349c9e50b987c6f58bb23324844527366d
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40635
Reviewed-by: Jason Lowe-Power 
Reviewed-by: Sandipan Das 
Reviewed-by: Pratik Sampat 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
A tests/test-progs/hello/bin/power/hello32
1 file changed, 0 insertions(+), 0 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, but someone else must approve; Looks  
good to me, approved

  Sandipan Das: Looks good to me, approved
  Pratik Sampat: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/tests/test-progs/hello/bin/power/hello32  
b/tests/test-progs/hello/bin/power/hello32

new file mode 100755
index 000..6619ae3
--- /dev/null
+++ b/tests/test-progs/hello/bin/power/hello32
Binary files differ

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40635
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: I77ef31349c9e50b987c6f58bb23324844527366d
Gerrit-Change-Number: 40635
Gerrit-PatchSet: 2
Gerrit-Owner: Boris Shingarov 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Boris Shingarov 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Pratik Sampat 
Gerrit-Reviewer: Sandipan Das 
Gerrit-Reviewer: kokoro 
Gerrit-CC: Gabe Black 
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]: sim: Add a void * analogue to VPtr.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40497 )


Change subject: sim: Add a void * analogue to VPtr.
..

sim: Add a void * analogue to VPtr.

The default type for VPtr is now void, and the void partial
specialization of VPtr is basically just a fancy container for Addr. Its
purpose is to distinguish guest addresses from actual uint64_t-s in the
signature of simcalls so that types which are purposefully 64 bits will
stay that way, and addresses will scale to the size of pointers in the
target ABI.

Change-Id: I71e2201f5917005861ba678c6675dbcbaa0965b3
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40497
Maintainer: Gabe Black 
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
---
M src/sim/proxy_ptr.hh
1 file changed, 27 insertions(+), 2 deletions(-)

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



diff --git a/src/sim/proxy_ptr.hh b/src/sim/proxy_ptr.hh
index 968c156..cd0d409 100644
--- a/src/sim/proxy_ptr.hh
+++ b/src/sim/proxy_ptr.hh
@@ -251,7 +251,8 @@
 explicit ProxyPtr(Args&&... args) : CPP(0, args...) {}

 template ::value>>
+typename std::enable_if_t::value &&
+  !std::is_same::value>>
 ProxyPtr(const ProxyPtr ) : CPP(other) {}

 ProxyPtr(const PP ) : CPP(other) {}
@@ -322,6 +323,30 @@
 }
 };

+template 
+class ProxyPtr
+{
+  protected:
+Addr _addr;
+
+  public:
+ProxyPtr(Addr new_addr, ...) : _addr(new_addr) {}
+
+template 
+ProxyPtr(const ProxyPtr ) : _addr(other.addr()) {}
+
+ProxyPtr &
+operator = (Addr new_addr)
+{
+_addr = new_addr;
+return *this;
+}
+
+operator Addr() const { return _addr; }
+
+Addr addr() const { return _addr; }
+};
+
 template 
 typename std::enable_if_t::value, ProxyPtr>
 operator + (A a, const ProxyPtr )
@@ -368,7 +393,7 @@

 template 
 using ConstVPtr = ConstProxyPtr;
-template 
+template 
 using VPtr = ProxyPtr;

 #endif // __SIM_PROXY_PTR_HH__

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40497
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: I71e2201f5917005861ba678c6675dbcbaa0965b3
Gerrit-Change-Number: 40497
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Jonathan Bohren 
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,sim: Add a UintPtr type to the ABI types for GuestABI.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40496 )


Change subject: arch,sim: Add a UintPtr type to the ABI types for GuestABI.
..

arch,sim: Add a UintPtr type to the ABI types for GuestABI.

This type is primarily used to determine the size of a pointer when
using that ABI, similar to the uintptr_t type, but also less directly
to determine the "native" size of the ABI. For instance, for 32 bit ARM
ABIs, it should be defined as uint32_t since that's both the size of a
uintptr_t, and, less directly, the size of a 32 bit ARM register and
"naturally" sized types in that ABI.

This type can be used by the VPtr template to retrieve its actual value
from a simcall's parameters. In general, when accepting or returning a
pointer or address in a simcall, the VPtr template should be used so
that it's managed correctly by GuestABI. Addr will be treated as a
uint64_t allways which will be incorrect for 32 bit ABIs.

Change-Id: I3af046917387541d6faff96a21a1f1dbf7317e06
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40496
Maintainer: Gabe Black 
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
---
M src/arch/arm/aapcs64.hh
M src/arch/arm/semihosting.hh
M src/sim/proxy_ptr.hh
M src/sim/proxy_ptr.test.cc
M src/sim/syscall_abi.hh
5 files changed, 17 insertions(+), 5 deletions(-)

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



diff --git a/src/arch/arm/aapcs64.hh b/src/arch/arm/aapcs64.hh
index 8a5e437..fb7b8f8 100644
--- a/src/arch/arm/aapcs64.hh
+++ b/src/arch/arm/aapcs64.hh
@@ -44,6 +44,8 @@

 struct Aapcs64
 {
+using UintPtr = uint64_t;
+
 struct State
 {
 int ngrn=0; // Next general purpose register number.
diff --git a/src/arch/arm/semihosting.hh b/src/arch/arm/semihosting.hh
index 74697f4..7566887 100644
--- a/src/arch/arm/semihosting.hh
+++ b/src/arch/arm/semihosting.hh
@@ -133,6 +133,8 @@

 struct Abi64 : public AbiBase
 {
+using UintPtr = uint64_t;
+
 class State : public StateBase
 {
   public:
@@ -145,6 +147,8 @@

 struct Abi32 : public AbiBase
 {
+using UintPtr = uint32_t;
+
 class State : public StateBase
 {
   public:
diff --git a/src/sim/proxy_ptr.hh b/src/sim/proxy_ptr.hh
index 02263ba..968c156 100644
--- a/src/sim/proxy_ptr.hh
+++ b/src/sim/proxy_ptr.hh
@@ -338,7 +338,8 @@
 static ProxyPtr
 get(ThreadContext *tc, typename ABI::State )
 {
-return ProxyPtr(Argument::get(tc, state), tc);
+return ProxyPtr(
+Argument::get(tc, state), tc);
 }
 };

@@ -349,7 +350,7 @@
 get(ThreadContext *tc, typename ABI::State )
 {
 return ConstProxyPtr(
-Argument::get(tc, state), tc);
+Argument::get(tc, state), tc);
 }
 };

diff --git a/src/sim/proxy_ptr.test.cc b/src/sim/proxy_ptr.test.cc
index b9f46e6..de0194d 100644
--- a/src/sim/proxy_ptr.test.cc
+++ b/src/sim/proxy_ptr.test.cc
@@ -465,6 +465,7 @@

 struct TestABI
 {
+using UintPtr = uint64_t;
 using State = int;
 };

diff --git a/src/sim/syscall_abi.hh b/src/sim/syscall_abi.hh
index 984f0e0..9d55202 100644
--- a/src/sim/syscall_abi.hh
+++ b/src/sim/syscall_abi.hh
@@ -54,10 +54,14 @@
 };

 struct GenericSyscallABI64 : public GenericSyscallABI
-{};
+{
+using UintPtr = uint64_t;
+};

 struct GenericSyscallABI32 : public GenericSyscallABI
 {
+using UintPtr = uint32_t;
+
 // Is this argument too big for a single register?
 template 
 struct IsWide;
@@ -65,7 +69,7 @@
 template 
 struct IsWide::value &&
-(sizeof(T) < sizeof(uint64_t) ||
+(sizeof(T) <= sizeof(UintPtr) ||
  GuestABI::IsConforming::value)>>
 {
 static const bool value = false;
@@ -74,7 +78,7 @@
 template 
 struct IsWide::value &&
-sizeof(T) == sizeof(uint64_t) &&
+(sizeof(T) > sizeof(UintPtr)) &&
 !GuestABI::IsConforming::value>>
 {
 static const bool value = true;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40496
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: I3af046917387541d6faff96a21a1f1dbf7317e06
Gerrit-Change-Number: 40496
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Jonathan Bohren 
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]: dev-arm: Reduce boilerplate when read/writing to Pio devices

[Giacomo Travaglini (Gerrit) via gem5-dev]

Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40775 )



Change subject: dev-arm: Reduce boilerplate when read/writing to Pio devices
..

dev-arm: Reduce boilerplate when read/writing to Pio devices

Change-Id: Id59ac950f37d7f4f2642daf324d501da1ee622de
Signed-off-by: Giacomo Travaglini 
---
M src/dev/arm/pl011.cc
M src/dev/arm/rtc_pl031.cc
M src/dev/arm/ufs_device.cc
3 files changed, 9 insertions(+), 70 deletions(-)



diff --git a/src/dev/arm/pl011.cc b/src/dev/arm/pl011.cc
index ea76416..cfe241d 100755
--- a/src/dev/arm/pl011.cc
+++ b/src/dev/arm/pl011.cc
@@ -64,6 +64,7 @@
 Pl011::read(PacketPtr pkt)
 {
 assert(pkt->getAddr() >= pioAddr && pkt->getAddr() < pioAddr +  
pioSize);

+assert(pkt->getSize() <= 4);

 Addr daddr = pkt->getAddr() - pioAddr;

@@ -145,22 +146,7 @@
 break;
 }

-switch(pkt->getSize()) {
-  case 1:
-pkt->setLE(data);
-break;
-  case 2:
-pkt->setLE(data);
-break;
-  case 4:
-pkt->setLE(data);
-break;
-  default:
-panic("Uart read size too big?\n");
-break;
-}
-
-
+pkt->setUintX(data, ByteOrder::little);
 pkt->makeAtomicResponse();
 return pioDelay;
 }
@@ -170,6 +156,7 @@
 {

 assert(pkt->getAddr() >= pioAddr && pkt->getAddr() < pioAddr +  
pioSize);

+assert(pkt->getSize() <= 4);

 Addr daddr = pkt->getAddr() - pioAddr;

@@ -179,23 +166,7 @@
 // use a temporary data since the uart registers are read/written with
 // different size operations
 //
-uint32_t data = 0;
-
-switch(pkt->getSize()) {
-  case 1:
-data = pkt->getLE();
-break;
-  case 2:
-data = pkt->getLE();
-break;
-  case 4:
-data = pkt->getLE();
-break;
-  default:
-panic("Uart write size too big?\n");
-break;
-}
-
+const uint32_t data = pkt->getUintX(ByteOrder::little);

 switch (daddr) {
 case UART_DR:
diff --git a/src/dev/arm/rtc_pl031.cc b/src/dev/arm/rtc_pl031.cc
index a6cdc7d..de84384 100644
--- a/src/dev/arm/rtc_pl031.cc
+++ b/src/dev/arm/rtc_pl031.cc
@@ -61,7 +61,7 @@
 PL031::read(PacketPtr pkt)
 {
 assert(pkt->getAddr() >= pioAddr && pkt->getAddr() < pioAddr +  
pioSize);

-assert(pkt->getSize() == 4);
+assert(pkt->getSize() <= 4);
 Addr daddr = pkt->getAddr() - pioAddr;
 uint32_t data;

@@ -99,22 +99,7 @@
 break;
 }

-switch(pkt->getSize()) {
-  case 1:
-pkt->setLE(data);
-break;
-  case 2:
-pkt->setLE(data);
-break;
-  case 4:
-pkt->setLE(data);
-break;
-  default:
-panic("Uart read size too big?\n");
-break;
-}
-
-
+pkt->setUintX(data, ByteOrder::little);
 pkt->makeAtomicResponse();
 return pioDelay;
 }
@@ -123,7 +108,7 @@
 PL031::write(PacketPtr pkt)
 {
 assert(pkt->getAddr() >= pioAddr && pkt->getAddr() < pioAddr +  
pioSize);

-assert(pkt->getSize() == 4);
+assert(pkt->getSize() <= 4);
 Addr daddr = pkt->getAddr() - pioAddr;
 DPRINTF(Timer, "Writing to RTC at offset: %#x\n", daddr);

diff --git a/src/dev/arm/ufs_device.cc b/src/dev/arm/ufs_device.cc
index 1406e4a..e04cb39 100644
--- a/src/dev/arm/ufs_device.cc
+++ b/src/dev/arm/ufs_device.cc
@@ -1029,26 +1029,9 @@
 Tick
 UFSHostDevice::write(PacketPtr pkt)
 {
-uint32_t data = 0;
+assert(pkt->getSize() <= 4);

-switch (pkt->getSize()) {
-
-  case 1:
-data = pkt->getLE();
-break;
-
-  case 2:
-data = pkt->getLE();
-break;
-
-  case 4:
-data = pkt->getLE();
-break;
-
-  default:
-panic("Undefined UFSHCD controller write size!\n");
-break;
-}
+const uint32_t data = pkt->getUintX(ByteOrder::little);

 switch (pkt->getAddr() & 0xFF)
 {

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40775
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: Id59ac950f37d7f4f2642daf324d501da1ee622de
Gerrit-Change-Number: 40775
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]: base: Initialize storage params on constructor

[Daniel Carvalho (Gerrit) via gem5-dev]

Daniel Carvalho has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/25426 )


Change subject: base: Initialize storage params on constructor
..

base: Initialize storage params on constructor

Force error checking on storage params by imposing it on construction.

Change-Id: I66a902cd5a7c809d3ac5be65b406de29fc0acf1c
Signed-off-by: Daniel R. Carvalho 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/25426
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Bobby R. Bruce 
---
M src/base/statistics.hh
M src/base/stats/storage.cc
M src/base/stats/storage.hh
M src/base/stats/storage.test.cc
4 files changed, 64 insertions(+), 116 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/base/statistics.hh b/src/base/statistics.hh
index b1f27c7..a1994c8 100644
--- a/src/base/statistics.hh
+++ b/src/base/statistics.hh
@@ -2005,14 +2005,7 @@
 Distribution &
 init(Counter min, Counter max, Counter bkt)
 {
-DistStor::Params *params = new DistStor::Params;
-params->min = min;
-params->max = max;
-params->bucket_size = bkt;
-// Division by zero is especially serious in an Aarch64 host,
-// where it gets rounded to allocate 32GiB RAM.
-assert(bkt > 0);
-params->buckets = (size_type)ceil((max - min + 1.0) / bkt);
+DistStor::Params *params = new DistStor::Params(min, max, bkt);
 this->setParams(params);
 this->doInit();
 return this->self();
@@ -2040,8 +2033,7 @@
 Histogram &
 init(size_type size)
 {
-HistStor::Params *params = new HistStor::Params;
-params->buckets = size;
+HistStor::Params *params = new HistStor::Params(size);
 this->setParams(params);
 this->doInit();
 return this->self();
@@ -2112,11 +2104,7 @@
 VectorDistribution &
 init(size_type size, Counter min, Counter max, Counter bkt)
 {
-DistStor::Params *params = new DistStor::Params;
-params->min = min;
-params->max = max;
-params->bucket_size = bkt;
-params->buckets = (size_type)ceil((max - min + 1.0) / bkt);
+DistStor::Params *params = new DistStor::Params(min, max, bkt);
 this->setParams(params);
 this->doInit(size);
 return this->self();
diff --git a/src/base/stats/storage.cc b/src/base/stats/storage.cc
index 02e8716..6e2a7f2 100644
--- a/src/base/stats/storage.cc
+++ b/src/base/stats/storage.cc
@@ -54,10 +54,7 @@
 else if (val > max_track)
 overflow += number;
 else {
-size_type index =
-(size_type)std::floor((val - min_track) / bucket_size);
-assert(index < size());
-cvec[index] += number;
+cvec[std::floor((val - min_track) / bucket_size)] += number;
 }

 if (val < min_val)
diff --git a/src/base/stats/storage.hh b/src/base/stats/storage.hh
index 66565bb..2ee1eb2 100644
--- a/src/base/stats/storage.hh
+++ b/src/base/stats/storage.hh
@@ -31,6 +31,7 @@
 #define __BASE_STATS_STORAGE_HH__

 #include 
+#include 

 #include "base/cast.hh"
 #include "base/logging.hh"
@@ -265,8 +266,19 @@
 /** The number of buckets. Equal to (max-min)/bucket_size. */
 size_type buckets;

-Params() : DistParams(Dist), min(0), max(0), bucket_size(0),
-   buckets(0) {}
+Params(Counter _min, Counter _max, Counter _bucket_size)
+  : DistParams(Dist), min(_min), max(_max),  
bucket_size(_bucket_size),

+buckets(0)
+{
+fatal_if(bucket_size <= 0,
+"Bucket size (%f) must be greater than zero", bucket_size);
+warn_if(std::floor((max - min + 1.0) / bucket_size) !=
+std::ceil((max - min + 1.0) / bucket_size),
+"Bucket size (%f) does not divide range [%f:%f] into  
equal-" \
+"sized buckets. Rounding up.", bucket_size, min + 1.0,  
max);

+
+buckets = std::ceil((max - min + 1.0) / bucket_size);
+}
 };

 DistStor(Info *info)
@@ -446,14 +458,18 @@
 /** The number of buckets. */
 size_type buckets;

-Params() : DistParams(Hist), buckets(0) {}
+Params(size_type _buckets)
+  : DistParams(Hist)
+{
+fatal_if(_buckets < 2,
+"There must be at least two buckets in a histogram");
+buckets = _buckets;
+}
 };

 HistStor(Info *info)
 : cvec(safe_cast(info->storageParams)->buckets)
 {
-fatal_if(cvec.size() == 1,
-"There must be at least two buckets in a histogram");
 reset(info);
 }

diff --git a/src/base/stats/storage.test.cc b/src/base/stats/storage.test.cc
index 8a4f6ed..78df23d 100644
--- a/src/base/stats/storage.test.cc
+++ 

[gem5-dev] Change in gem5/gem5[develop]: base: Fix storage params safe_cast

[Daniel Carvalho (Gerrit) via gem5-dev]

Daniel Carvalho has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40555 )


Change subject: base: Fix storage params safe_cast
..

base: Fix storage params safe_cast

Although they provide the exact same behavior, the params
created in the tests did not have the type expected by the
internal safe cast.

The following error was triggered:

storage.test.debug: build/NULL/base/cast.hh:47: T safe_cast(U)
 [with T = const Stats::SampleStor::Params*;
  U = const Stats::StorageParams*]:
 Assertion `ret' failed.

Change-Id: I4f2ba51f3ccdb44589e61f235997245e7d9bf3c9
Signed-off-by: Daniel R. Carvalho 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40555
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Bobby R. Bruce 
---
M src/base/stats/storage.test.cc
1 file changed, 4 insertions(+), 4 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/base/stats/storage.test.cc b/src/base/stats/storage.test.cc
index 717c881..8a4f6ed 100644
--- a/src/base/stats/storage.test.cc
+++ b/src/base/stats/storage.test.cc
@@ -1083,7 +1083,7 @@
 Stats::Counter val;
 Stats::DistData data;
 Stats::DistData expected_data;
-Stats::DistParams params(Stats::Deviation);
+Stats::SampleStor::Params params;
 MockInfo info();

 // Simple test with one value being sampled
@@ -1133,7 +1133,7 @@
 Stats::Counter val = 10;
 Stats::Counter num_samples = 5;
 Stats::DistData data;
-Stats::DistParams params(Stats::Deviation);
+Stats::SampleStor::Params params;
 MockInfo info();

 ASSERT_EQ(stor.size(), 1);
@@ -1177,7 +1177,7 @@
 Stats::Counter val;
 Stats::DistData data;
 Stats::DistData expected_data;
-Stats::DistParams params(Stats::Deviation);
+Stats::AvgSampleStor::Params params;
 MockInfo info();

 // Simple test with one value being sampled
@@ -1228,7 +1228,7 @@
 Stats::Counter val = 10;
 Stats::Counter num_samples = 5;
 Stats::DistData data;
-Stats::DistParams params(Stats::Deviation);
+Stats::AvgSampleStor::Params params;
 MockInfo info();

 ASSERT_EQ(stor.size(), 1);



The change was submitted with unreviewed changes in the following files:

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40555
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: I4f2ba51f3ccdb44589e61f235997245e7d9bf3c9
Gerrit-Change-Number: 40555
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Carvalho 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
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] Build failed in Jenkins: Weekly #2

[jenkins-no-reply--- via gem5-dev]

See 

Changes:

[cuijin7] arch-riscv: fix unintentionally CSR bit overwritten in different mode

[gabe.black] base: Style fixes in base/refcnt.hh

[gabe.black] arch-arm: Fix style in decoder.hh.

[Giacomo Travaglini] configs: Use MmioVirtIO for disk image in baremetal.py

[gabe.black] ext: Replace Queue.Empty with queue.empty

[gabe.black] sim: Eliminate the generic PseudoInstABI.

[kyleroarty1716] dev-hsa: enable interruptible hsa signal support

[gabe.black] arch: Correct style in the ISA base class.

[gabe.black] arch: Stop using switching header files in ISA specific files.

[kyleroarty1716] dev-hsa: Add missing include to hsa_driver.hh

[gabe.black] arch-x86: Fix style in arch/x86/types.hh.

[yuhsingw] systemc: set Gem5ToTlmBridge blockingRrequest with TLM_UPDATE 
returning

[shunhsingou] util: Fix packet parser for Python3

[shunhsingou] fastmodel: remove incorrect cntfrq update

[richard.cooper] util: Update util/gem5img.py to work with Python 3.

[richard.cooper] util: Improve robustness of sfdisk parsing in util/gem5img.py

[richard.cooper] util: Fix gem5img when used to manually unmount a disk image.

[gabe.black] arch-power: Delete unused register related constants.

[shunhsingou] fastmodel: create base class for EVS CPU

[shunhsingou] fastmodel: add interface to update system counter freq

[gabe.black] ext: Update pybind11 to version 2.6.2.

[adrian.herrera] arch-arm: don't expose FEAT_VHE by default

[shunhsingou] systemc: remove boost header dependency

[Giacomo Travaglini] ext: testlib loading tests from multiple directories

[Bobby R. Bruce] util,python: Fix Pre-commit hooks to ignore non-source files

[Bobby R. Bruce] util,python: Add check to ensure files are utf-8 in pre-commit

[odanrc] scons: Separate debug flags from debug-format flags

[odanrc] sim: Move cur tick to its own files

[odanrc] base,tests: Add a basic fake class to handle curTick

[odanrc] base: Move Stats::Info functions to its own source file

[odanrc] base,tests: Create unit tests for Stats::Stor

[gabe.black] arch: Templatize the BasicDecodeCache.

[gabe.black] arch-arm,cpu: Introduce a getEMI virtual method on StaticInst.

[gabe.black] arch-arm,cpu: Use getEMI() in more places.

[odanrc] scons: Add an "All" compound debug flag

[gabe.black] misc: Re-remove Authors lines from source files.

[Giacomo Travaglini] arch-arm: Add destRegIdxArr arrays to TME instructions

[Bobby R. Bruce] tests: Increase presubmit (Kokoro) timeout to 6 hours

[Bobby R. Bruce] arch-riscv,misc: Fix clang missing override errors

[Bobby R. Bruce] gpu-compute,misc: Fix Clang missing override errors

[Bobby R. Bruce] gpu-compute,misc: Remove unused private variable

[Bobby R. Bruce] misc: Updated the RELEASE-NOTES and version number

[Bobby R. Bruce] scons,python: Fix `--without-python` flag

[Bobby R. Bruce] tests: Changed 'long' boot tests to X86 from GCN3_X86

[mattdsinclair] arch-x86: Make JRCXZ instruction do 64-bit jump

[mattdsinclair] arch-gcn3: Implementation of s_sleep

[shunhsingou] fastmodel: fix cntfrq in A76

[Bobby R. Bruce] misc: Revert version info for develop branch

[gabe.black] cpu: Style fixes in the trace CPU.

[gabe.black] cpu: Replace fixed sized arrays in the O3 inst with variable 
arrays.

[gabe.black] arch,cpu: Move a Decode DPRINTF into the arch Decoder classes.


--
[...truncated 789.12 KB...]
 [   SHCXX] nomali/lib/jobslot.cc -> .os
 [   SHCXX] nomali/lib/mali_midgard.cc -> .os
 [   SHCXX] nomali/lib/mali_t6xx.cc -> .os
 [   SHCXX] nomali/lib/mali_t7xx.cc -> .os
 [   SHCXX] nomali/lib/addrspace.cc -> .os
 [   SHCXX] nomali/lib/mmu.cc -> .os
 [   SHCXX] nomali/lib/nomali_api.cc -> .os
 [   SHCXX] drampower/src/CommandAnalysis.cc -> .os
 [  AR]  -> nomali/libnomali.a
 [  RANLIB]  -> nomali/libnomali.a
 [LINK]  -> GCN3_X86/systemc/channel/lib.o.partial
 [   SHCXX] drampower/src/MemArchitectureSpec.cc -> .os
 [   SHCXX] drampower/src/MemCommand.cc -> .os
 [   SHCXX] drampower/src/MemPowerSpec.cc -> .os
 [   SHCXX] drampower/src/MemTimingSpec.cc -> .os
 [   SHCXX] drampower/src/MemoryPowerModel.cc -> .os
 [   SHCXX] drampower/src/MemorySpecification.cc -> .os
 [   SHCXX] drampower/src/Parameter.cc -> .os
 [   SHCXX] drampower/src/Parametrisable.cc -> .os
 [   SHCXX] drampower/src/libdrampower/LibDRAMPower.cc -> .os
 [   SHCXX] drampower/src/CAHelpers.cc -> .os
 [   SHCXX] drampower/src/CmdHandlers.cc -> .os
 [   SHCXX] drampower/src/MemBankWiseParams.cc -> .os
 [SHCC] libelf/elf_begin.c -> .os
 [SHCC] libelf/elf_cntl.c -> .os
 [SHCC] libelf/elf_data.c -> .os
 [SHCC] libelf/elf_end.c -> .os
 [SHCC] libelf/elf_errmsg.c -> .os
 [SHCC] libelf/elf_errno.c -> .os
 [SHCC] libelf/elf_fill.c -> .os
 [SHCC] libelf/elf_flag.c -> .os
 [SHCC] libelf/elf_getarhdr.c -> .os
 [SHCC] libelf/elf_getarsym.c -> .os
 [SHCC] libelf/elf_getbase.c -> .os
 [SHCC] 

[gem5-dev] Change in gem5/gem5[develop]: base: Fix copyright of base/stats/SConscript

[Daniel Carvalho (Gerrit) via gem5-dev]

Daniel Carvalho has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40755 )



Change subject: base: Fix copyright of base/stats/SConscript
..

base: Fix copyright of base/stats/SConscript

The original copyright was accidentally removed by
e59557af500d1633b1f41b023d6c072acaf145a0.

Change-Id: Ib3b7a34ea889fbd9a6f2a8e21a6bb24432939da9
Signed-off-by: Daniel R. Carvalho 
---
M src/base/stats/SConscript
1 file changed, 1 insertion(+), 0 deletions(-)



diff --git a/src/base/stats/SConscript b/src/base/stats/SConscript
index 69ac275..8f037ba 100644
--- a/src/base/stats/SConscript
+++ b/src/base/stats/SConscript
@@ -1,6 +1,7 @@
 # -*- mode:python -*-
 #
 # Copyright (c) 2021 Daniel R. Carvalho
+# Copyright (c) 2006 The Regents of The University of Michigan
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40755
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: Ib3b7a34ea889fbd9a6f2a8e21a6bb24432939da9
Gerrit-Change-Number: 40755
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Carvalho 
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]: base: Make read-only functions const in ScalarBase

[Daniel Carvalho (Gerrit) via gem5-dev]

Daniel Carvalho has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/27084 )


Change subject: base: Make read-only functions const in ScalarBase
..

base: Make read-only functions const in ScalarBase

These functions do not need to modify their storage's contents.

ScalarBase's non-const value() has been removed.

Change-Id: I4dd3899a29a741a7d8cd199ccd254b346d86ae07
Signed-off-by: Daniel R. Carvalho 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/27084
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Bobby R. Bruce 
---
M src/base/statistics.hh
1 file changed, 8 insertions(+), 11 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/base/statistics.hh b/src/base/statistics.hh
index 1c3d53d..b1f27c7 100644
--- a/src/base/statistics.hh
+++ b/src/base/statistics.hh
@@ -523,13 +523,6 @@
 }

   public:
-/**
- * Return the current value of this stat as its base type.
- * @return The current value.
- */
-Counter value() const { return data()->value(); }
-
-  public:
 ScalarBase(Group *parent = nullptr, const char *name = nullptr,
const char *desc = nullptr)
 : DataWrap(parent, name, desc)
@@ -585,13 +578,17 @@
  */
 size_type size() const { return 1; }

-Counter value() { return data()->value(); }
+/**
+ * Return the current value of this stat as its base type.
+ * @return The current value.
+ */
+Counter value() const { return data()->value(); }

-Result result() { return data()->result(); }
+Result result() const { return data()->result(); }

-Result total() { return result(); }
+Result total() const { return result(); }

-bool zero() { return result() == 0.0; }
+bool zero() const { return result() == 0.0; }

 void reset() { data()->reset(this->info()); }
 void prepare() { data()->prepare(this->info()); }



4 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/+/27084
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: I4dd3899a29a741a7d8cd199ccd254b346d86ae07
Gerrit-Change-Number: 27084
Gerrit-PatchSet: 13
Gerrit-Owner: Daniel Carvalho 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gem5 Cloud Project GCB service account  
<345032938...@cloudbuild.gserviceaccount.com>

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] build dir size, build time, and partial linking

[Gabe Black via gem5-dev]

Hi folks. As a result of this CL:

https://gem5-review.googlesource.com/c/public/gem5/+/40621

I've been looking at the size of the build directory, how long builds take,
and partial linking, aka a potential step in our build where we link
together some .o files into a larger .o, and then eventually link all those
.o files into the final binary.

You can get most of the details in the comment thread on that CL, but let
me first talk about partial linking. I had originally implemented that
because I was under the impression that the time it takes to link scales
super linearly (which I think is actually true), and that by doing smaller
links first, the scaling would work out where the total time spent linking
would go down, especially since the smaller links could be done in
parallel. Measurements have shown that that is *not* the case, and the
intermediate object files take up a lot of space. Since there is really no
benefit to partial linking, the implementation in gcc at least has been
plagued with bugs, and it has a significant cost in disk space and build
process complexity, I think we should get rid of it.

Next, I looked at trying to reduce build directory size since, as Jason
pointed out, if you have a slow disk (network storage?) then that can be a
major bottleneck. One trick I implemented was to enable a feature of gcc
and clang where they can compress the debug information throughout the
build. This significantly reduces the build directory size (guestimate by
1/3-1/2?). My computer is fairly new, has a pretty fast NVME SSD, and tons
of RAM for disk cache, and so the dominant factor for me seems to be the
compression time, and while the disk footprint is GBs lower, the build time
is still a good bit longer with this approach. If I was trying to shove a
few GB less over a network then I think this might be reversed. I think
this mechanism should be optional, especially since it's complexity cost is
low on the scons side.

Just a quick note about choice of linker. With the compression turned on,
gnu's ld seems to be the fastest, with gold and lld (clang's linker)
trailing by 10-20% total build time. This may change with the compression
turned off, but (to my surprise), these alternative linkers don't seem to
help, at least on my machine.

Finally, *more than half* of the build directory's size is taken up by the
python/_m5 directory where the pybind11 bindings for the params structs
live. The opportunity for improvement here is huge, although somewhat out
of our hands. It sounds like Ciro is working with upstream to hopefully
give us some alternatives here, but this is definitely the next thing to
look at as far as bringing down build directory size bloat.

Gabe
___
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: Upstreaming power-gem5

[Gabe Black via gem5-dev]

Re my commit, please feel free to add back the constants you need. I'm
probably going to try splitting up the ISA constants consumed by non-ISA
code from the ones that are internally used in registers.hh since I'm
trying to get rid of the former, but if that same constant is useful
*inside* an ISA then that's ok, I just want to keep them separated so one
doesn't leak back into the other over time.

Gabe

On Thu, Feb 4, 2021 at 8:39 PM Sandipan Das via gem5-dev 
wrote:

> Hello Boris,
>
> On 04/02/21 10:08 pm, Boris Shingarov wrote:
> >  > The current sequence breaks 32-bit support in
> >  > the beginning and then restores it back towards the end.
> >  > Wondering if that could be a problem with the CI?
> >
> > I would be surprised if there is even any POWER-specific CI at all.
> > The one POWER binary we had (in test-progs), was removed at c1ebdf66f.
> I've
> > been waiting on 86222736e (which just got in) before submitting
> > https://gem5-review.googlesource.com/c/public/gem5/+/40635
> >  ,
> > could you please code-review that?  Then, we are ready to merge your
> e52dbcb.
>
> Done.
>
> >
> >  > The current sequence breaks 32-bit support in
> >  > the beginning and then restores it back towards the end.
> >
> > Up to you really.  My guess is that when you look at how much `develop`
> has
> > diverged in the past months, you will find keeping the sequence less of
> a thing.
> >
>
> Got it. I started rebasing against 'develop' last night and ran
> into a problem with one of the branch instruction related patches.
> Hopefully, I can get that fixed soon. I also noticed this recent
> patch from Gabe:
>
> commit 7bb456f02
> Author: Gabe Black 
> Date:   Sun Jan 24 23:16:43 2021 -0800
>
> arch-power: Delete unused register related constants.
>
> I was actually planning on reusing some of these constants
> like here:
>
> https://github.com/sandip4n/gem5/commit/6ffda6f4583054a9d49b90cbd67189c813bd4dae
>
> Maybe I'll add a patch that reintroduces the relevant ones.
>
>
> - Sandipan
> ___
> 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 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]: tests: Delete the now unused unittest/unittest.[cc|hh].

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40618 )


Change subject: tests: Delete the now unused unittest/unittest.[cc|hh].
..

tests: Delete the now unused unittest/unittest.[cc|hh].

These files were originally used to provide a more gtest like mechanism
for the UnitTest executables, many of which didn't actually test
anything. With the definitions in those files, the tests could check
whether their expectations were met, and either pass or fail without a
human having to inspect the output and knowing what output to expect.

Change-Id: Ie0601391b994859eb544b37201333838fa3ba02a
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40618
Tested-by: kokoro 
Reviewed-by: Daniel Carvalho 
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
---
M src/unittest/SConscript
D src/unittest/unittest.cc
D src/unittest/unittest.hh
3 files changed, 0 insertions(+), 208 deletions(-)

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



diff --git a/src/unittest/SConscript b/src/unittest/SConscript
index b0e29ed..9ebe863 100644
--- a/src/unittest/SConscript
+++ b/src/unittest/SConscript
@@ -28,8 +28,6 @@

 Import('*')

-Source('unittest.cc')
-
 UnitTest('nmtest', 'nmtest.cc')

 stattest_py = PySource('m5', 'stattestmain.py', tags='stattest')
diff --git a/src/unittest/unittest.cc b/src/unittest/unittest.cc
deleted file mode 100644
index 2f99eae..000
--- a/src/unittest/unittest.cc
+++ /dev/null
@@ -1,92 +0,0 @@
-/*
- * Copyright (c) 2011 Advanced Micro Devices, Inc.
- * 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.
- */
-
-#include "unittest/unittest.hh"
-
-#include 
-
-#include "base/cprintf.hh"
-
-namespace {
-
-bool _printOnPass = (getenv("PRINT_ON_PASS") != NULL);
-unsigned _passes = 0;
-unsigned _failures = 0;
-
-bool _casePrinted = false;
-const char *_case = NULL;
-
-} // anonymous namespace
-
-namespace UnitTest {
-
-void
-checkVal(const char *file, const unsigned line,
- const char *test, const bool result)
-{
-if (!result || _printOnPass) {
-if (!_casePrinted && _case) {
-cprintf("CASE %s:\n", _case);
-_casePrinted = true;
-}
-cprintf("   CHECK %s:   %s:%d   %s\n",
-result ? "PASSED" : "FAILED", file, line, test);
-}
-if (result) _passes++;
-else _failures++;
-}
-
-bool printOnPass() { return _printOnPass; }
-void printOnPass(bool newPrintOnPass) { _printOnPass = newPrintOnPass; }
-
-unsigned passes() { return _passes; }
-unsigned failures() { return _failures; }
-
-unsigned
-printResults()
-{
-cprintf("TEST %s:   %d checks passed, %d checks failed.\n",
-_failures ? "FAILED" : "PASSED", _passes, _failures);
-return _failures;
-}
-
-void
-reset()
-{
-_passes = 0;
-_failures = 0;
-}
-
-void
-setCase(const char *newCase)
-{
-_casePrinted = false;
-_case = newCase;
-}
-
-} //namespace UnitTest
diff --git a/src/unittest/unittest.hh b/src/unittest/unittest.hh
deleted file mode 100644
index 5e9c432..000
--- a/src/unittest/unittest.hh
+++ /dev/null
@@ -1,114 +0,0 @@
-/*
- * Copyright (c) 2011 Advanced Micro Devices, Inc.
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the 

[gem5-dev] Change in gem5/gem5[develop]: base,tests: Convert cprintftime from a "UnitTest" to a normal bin.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40617 )


Change subject: base,tests: Convert cprintftime from a "UnitTest" to a  
normal bin.

..

base,tests: Convert cprintftime from a "UnitTest" to a normal bin.

This "UnitTest" was really not a unit test, it was a timing utility for
measuring the performance of gem5's cprintf implementation. The name was
misleading, but more than that, it was linked against all of gem5 which
created a approximately 1.5 gigabyte binary for what is a very small
program.

Instead, the new version of cprintftime, which has the same
functionality as the old version, weighs in at a svelte 500k with debug
information.

This also trims down the number of misleading "UnitTest" entries to 3,
getting us closer to the point where we can eliminate that type of
entity entirely.

Change-Id: Id30d094f2844e948fe67e820c89412f8667aaa52
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40617
Maintainer: Gabe Black 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
Reviewed-by: Daniel Carvalho 
Reviewed-by: Jason Lowe-Power 
---
M src/base/SConscript
R src/base/cprintftime.cc
M src/unittest/SConscript
3 files changed, 1 insertion(+), 1 deletion(-)

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



diff --git a/src/base/SConscript b/src/base/SConscript
index f8cd4ba..1190b93 100644
--- a/src/base/SConscript
+++ b/src/base/SConscript
@@ -39,6 +39,7 @@
 Source('channel_addr.cc')
 Source('cprintf.cc', add_tags='gtest lib')
 GTest('cprintf.test', 'cprintf.test.cc')
+Executable('cprintftime', 'cprintftime.cc', 'cprintf.cc')
 Source('debug.cc')
 GTest('debug.test', 'debug.test.cc', 'debug.cc')
 if env['USE_FENV']:
diff --git a/src/unittest/cprintftime.cc b/src/base/cprintftime.cc
similarity index 100%
rename from src/unittest/cprintftime.cc
rename to src/base/cprintftime.cc
diff --git a/src/unittest/SConscript b/src/unittest/SConscript
index 19b2542..b0e29ed 100644
--- a/src/unittest/SConscript
+++ b/src/unittest/SConscript
@@ -30,7 +30,6 @@

 Source('unittest.cc')

-UnitTest('cprintftime', 'cprintftime.cc')
 UnitTest('nmtest', 'nmtest.cc')

 stattest_py = PySource('m5', 'stattestmain.py', tags='stattest')



1 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/+/40617
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: Id30d094f2844e948fe67e820c89412f8667aaa52
Gerrit-Change-Number: 40617
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
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]: scons: Remove the "abstract" tag from Executable classes.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40616 )


Change subject: scons: Remove the "abstract" tag from Executable classes.
..

scons: Remove the "abstract" tag from Executable classes.

That tag was intended to mark an Executable subclass as abstract, aka
only suitable for using as bases for other Executable subclasses and not
for direct instantiation. The only place it was used was the base
Executable class however, and that class is actually directly useful
when setting up a generic executable from other gem5 sources.

Change-Id: I70204b63c03bb45bf21b8c312a7b8581be5e0cab
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40616
Tested-by: kokoro 
Reviewed-by: Daniel Carvalho 
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
---
M src/SConscript
1 file changed, 1 insertion(+), 5 deletions(-)

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



diff --git a/src/SConscript b/src/SConscript
index 74b9516..538d8aa 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -477,17 +477,13 @@
 all = []

 def __init__(cls, name, bases, d):
-if not d.pop('abstract', False):
-ExecutableMeta.all.append(cls)
+ExecutableMeta.all.append(cls)
 super(ExecutableMeta, cls).__init__(name, bases, d)
-
 cls.all = []

 class Executable(object, metaclass=ExecutableMeta):
 '''Base class for creating an executable from sources.'''

-abstract = True
-
 def __init__(self, target, *srcs_and_filts):
 '''Specify the target name and any sources. Sources that are
 not SourceFiles are evalued with Source().'''



1 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/+/40616
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: I70204b63c03bb45bf21b8c312a7b8581be5e0cab
Gerrit-Change-Number: 40616
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
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]: base: Replace a "panic" in cprintf with an M5_UNREACHABLE.

[Gabe Black (Gerrit) via gem5-dev]

Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40615 )


Change subject: base: Replace a "panic" in cprintf with an M5_UNREACHABLE.
..

base: Replace a "panic" in cprintf with an M5_UNREACHABLE.

The panic was just to signal that a point in the code should be
unreachable, and brought with it a thread of dependencies which would
bring in more and more extra files as it was followed.

Change-Id: I46fb99b91929dca78a6547bdc7635aab9a63a9f3
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40615
Tested-by: kokoro 
Reviewed-by: Daniel Carvalho 
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
---
M src/base/cprintf.cc
1 file changed, 1 insertion(+), 2 deletions(-)

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



diff --git a/src/base/cprintf.cc b/src/base/cprintf.cc
index 03fa3cb..7a3e958 100644
--- a/src/base/cprintf.cc
+++ b/src/base/cprintf.cc
@@ -33,7 +33,6 @@
 #include 

 #include "base/compiler.hh"
-#include "base/logging.hh"

 namespace cp
 {
@@ -239,7 +238,7 @@
 break;

   case '%':
-panic("we shouldn't get here");
+M5_UNREACHABLE;
 break;

   default:



1 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/+/40615
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: I46fb99b91929dca78a6547bdc7635aab9a63a9f3
Gerrit-Change-Number: 40615
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
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