[gem5-dev] Change in gem5/gem5[develop]: stdlib,mem-ruby: CHI support in components

2021-11-04 Thread Jason Lowe-Power (Gerrit) via gem5-dev
Jason Lowe-Power has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/52463 )



Change subject: stdlib,mem-ruby: CHI support in components
..

stdlib,mem-ruby: CHI support in components

This changeset adds CHI support in the components library. Currently,
only a very simple one level protocol is implemented, but hopefully this
design will be able to scale to other more complex hierarchies.

I've tested this with RISC-V with 1 and 4 cores and with x86 with 1
core. Since we don't have an Arm-compatible board, I haven't tested with
ARM. Note that x86 with more than 1 core boots most of the way, but it
hangs during systemd (the kernel comes up completely).

Change-Id: I56953238c6b0ca5ac754b103a1b6ec05a85a0af5
Signed-off-by: Jason Lowe-Power 
---
A src/python/gem5/components/cachehierarchies/chi/nodes/abstract_node.py
A src/python/gem5/components/cachehierarchies/chi/nodes/dma_requestor.py
A  
src/python/gem5/components/cachehierarchies/chi/nodes/private_l1_moesi_cache.py

M src/python/SConscript
A src/python/gem5/components/cachehierarchies/chi/nodes/__init__.py
A src/python/gem5/components/cachehierarchies/chi/nodes/memory_controller.py
A src/python/gem5/components/cachehierarchies/chi/__init__.py
A src/python/gem5/components/cachehierarchies/chi/nodes/directory.py
A  
src/python/gem5/components/cachehierarchies/chi/private_l1_cache_hierarchy.py

9 files changed, 750 insertions(+), 0 deletions(-)



diff --git a/src/python/SConscript b/src/python/SConscript
index e750829..c3c3374 100644
--- a/src/python/SConscript
+++ b/src/python/SConscript
@@ -46,6 +46,22 @@
 'gem5/components/cachehierarchies/abstract_cache_hierarchy.py')
 PySource('gem5.components.cachehierarchies',
 'gem5/components/cachehierarchies/abstract_two_level_cache_hierarchy.py')
+PySource('gem5.components.cachehierarchies.chi',
+'gem5/components/cachehierarchies/chi/__init__.py')
+PySource('gem5.components.cachehierarchies.chi',
+'gem5/components/cachehierarchies/chi/private_l1_cache_hierarchy.py')
+PySource('gem5.components.cachehierarchies.chi.nodes',
+'gem5/components/cachehierarchies/chi/nodes/__init__.py')
+PySource('gem5.components.cachehierarchies.chi.nodes',
+'gem5/components/cachehierarchies/chi/nodes/abstract_node.py')
+PySource('gem5.components.cachehierarchies.chi.nodes',
+'gem5/components/cachehierarchies/chi/nodes/directory.py')
+PySource('gem5.components.cachehierarchies.chi.nodes',
+'gem5/components/cachehierarchies/chi/nodes/dma_requestor.py')
+PySource('gem5.components.cachehierarchies.chi.nodes',
+'gem5/components/cachehierarchies/chi/nodes/private_l1_moesi_cache.py')
+PySource('gem5.components.cachehierarchies.chi.nodes',
+'gem5/components/cachehierarchies/chi/nodes/memory_controller.py')
 PySource('gem5.components.cachehierarchies.classic',
 'gem5/components/cachehierarchies/classic/__init__.py')
 PySource('gem5.components.cachehierarchies.classic',
diff --git a/src/python/gem5/components/cachehierarchies/chi/__init__.py  
b/src/python/gem5/components/cachehierarchies/chi/__init__.py

new file mode 100644
index 000..e69de29
--- /dev/null
+++ b/src/python/gem5/components/cachehierarchies/chi/__init__.py
diff --git  
a/src/python/gem5/components/cachehierarchies/chi/nodes/__init__.py  
b/src/python/gem5/components/cachehierarchies/chi/nodes/__init__.py

new file mode 100644
index 000..e69de29
--- /dev/null
+++ b/src/python/gem5/components/cachehierarchies/chi/nodes/__init__.py
diff --git  
a/src/python/gem5/components/cachehierarchies/chi/nodes/abstract_node.py  
b/src/python/gem5/components/cachehierarchies/chi/nodes/abstract_node.py

new file mode 100644
index 000..4702ca0
--- /dev/null
+++ b/src/python/gem5/components/cachehierarchies/chi/nodes/abstract_node.py
@@ -0,0 +1,129 @@
+# Copyright (c) 2021 The Regents of the University of California
+# 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 

[gem5-dev] Re: Build failed in Jenkins: nightly #27

2021-11-04 Thread Gabe Black via gem5-dev
Oh, yes, that does look very suspicious. I'll have to take a closer look at
that!

Gabe

On Thu, Nov 4, 2021 at 3:02 PM Jason Lowe-Power  wrote:

> Glad you asked! I didn't look closely enough at the output.
>
> Here's an error that looks suspicious (The whole file is attached.)
>
> build/ARM_clang/cpu/o3/dyn_inst.hh:252:29: runtime error: constructor call
> on misaligned address 0x1c1ed9ac for type 'gem5::PhysRegId *', which
> requires 8 byte alignment
> 0x1c1ed9ac: note: pointer points here
>   00 00 00 00 00 00 00 00  50 d9 1e 1c 00 00 00 00  70 8d 20 1c 00 00 00
> 00  40 58 1e 1c 00 00 00 00
>   ^
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> build/ARM_clang/cpu/o3/dyn_inst.hh:252:29 in
> build/ARM_clang/cpu/o3/dyn_inst.hh:335:13: runtime error: store to
> misaligned address 0x1c1ed9bc for type 'gem5::PhysRegIdPtr' (aka
> 'gem5::PhysRegId *'), which requires 8 byte alignment
> 0x1c1ed9bc: note: pointer points here
>   70 8d 20 1c 00 00 00 00  40 58 1e 1c 00 00 00 00  1a 00 00 00 00 00 00
> 00  1a 00 00 00 00 00 00 00
>   ^
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> build/ARM_clang/cpu/o3/dyn_inst.hh:335:13 in
> build/ARM_clang/cpu/o3/dyn_inst.hh:307:13: runtime error: store to
> misaligned address 0x1c1ed9ac for type 'gem5::PhysRegIdPtr' (aka
> 'gem5::PhysRegId *'), which requires 8 byte alignment
> 0x1c1ed9ac: note: pointer points here
>   00 00 00 00 00 00 00 00  50 d9 1e 1c 00 00 00 00  70 8d 20 1c 00 00 fb
> 1c  00 00 00 00 80 10 fb 1c
>   ^
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> build/ARM_clang/cpu/o3/dyn_inst.hh:307:13 in
> build/ARM_clang/cpu/o3/dyn_inst.hh:322:13: runtime error: store to
> misaligned address 0x1c1ed9b4 for type 'gem5::PhysRegIdPtr' (aka
> 'gem5::PhysRegId *'), which requires 8 byte alignment
> 0x1c1ed9b4: note: pointer points here
>   00 00 00 00 00 00 00 00  70 8d 20 1c 00 00 fb 1c  00 00 00 00 80 10 fb
> 1c  00 00 00 00 50 0d fb 1c
>   ^
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> build/ARM_clang/cpu/o3/dyn_inst.hh:322:13 in
> build/ARM_clang/cpu/o3/dyn_inst.hh:300:20: runtime error: load of
> misaligned address 0x1c1ed9ac for type 'gem5::PhysRegIdPtr' (aka
> 'gem5::PhysRegId *'), which requires 8 byte alignment
> 0x1c1ed9ac: note: pointer points here
>   00 00 00 00 f0 8f 01 18  00 00 00 00 c0 8c 01 18  00 00 00 00 00 00 fb
> 1c  00 00 00 00 80 10 fb 1c
>   ^
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> build/ARM_clang/cpu/o3/dyn_inst.hh:300:20 in
> build/ARM_clang/cpu/o3/dyn_inst.hh:329:20: runtime error: load of
> misaligned address 0x1d223754 for type 'gem5::PhysRegIdPtr' (aka
> 'gem5::PhysRegId *'), which requires 8 byte alignment
> 0x1d223754: note: pointer points here
>   00 00 00 00 f0 8f 01 18  00 00 00 00 18 60 01 1d  00 00 00 00 78 60 01
> 1d  00 00 00 00 78 60 01 1d
>   ^
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> build/ARM_clang/cpu/o3/dyn_inst.hh:329:20 in
> build/ARM_clang/cpu/o3/dyn_inst.hh:315:20: runtime error: load of
> misaligned address 0x1c1ecba4 for type 'gem5::PhysRegIdPtr' (aka
> 'gem5::PhysRegId *'), which requires 8 byte alignment
> 0x1c1ecba4: note: pointer points here
>   00 00 00 00 f8 90 01 18  00 00 00 00 f8 90 01 18  00 00 00 00 c0 60 01
> 1d  00 00 00 00 78 60 01 1d
>   ^
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> build/ARM_clang/cpu/o3/dyn_inst.hh:315:20 in
>
>
> Cheers,
> Jason
>
> On Thu, Nov 4, 2021 at 2:51 PM Gabe Black  wrote:
>
>> Did you find anything with ASAN?
>>
>> Gabe
>>
>> On Thu, Nov 4, 2021, 12:59 PM Gabe Black  wrote:
>>
>>> I don't know if they do, and frankly even the unique_ptr change could
>>> since that could fix heap corruption, even though it's unlikely since I
>>> don't think this regression uses any of that code (except maybe VIO). We
>>> don't actually *know* that that change is at fault, even though I agree
>>> that it seems like a reasonable guess. This could be one of those bugs
>>> where something totally unrelated is wrong, and another change just shifts
>>> things around until some address lines up with something bad.
>>>
>>> As valgrind grinds on, I think there's a good chance of identifying
>>> exactly what the problem is. I think that should happen in the next day or
>>> two, hopefully.
>>>
>>> Gabe
>>>
>>> On Thu, Nov 4, 2021 at 9:55 AM Jason Lowe-Power 
>>> wrote:
>>>
 Hey Gabe,

 Do these fix the nightly regression? If not, we may need to back out
 "4fe56ff72 - (3 months ago) arch-arm,cpu: Replace rename modes with split
 reg/elem register files." until we have a fix.

 Cheers,
 Jason

 On Thu, Nov 4, 2021 at 12:13 AM Gabe Black 
 wrote:

> Valgrind hasn't finished, but what it found so far is attached. I went
> through it and have the following changes which should address these
> 

[gem5-dev] Change in gem5/gem5[develop]: arch-gcn3,arch-vega: Don't write exec in v_cmp_f_i32

2021-11-04 Thread Kyle Roarty (Gerrit) via gem5-dev
Kyle Roarty has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/52445 )



Change subject: arch-gcn3,arch-vega: Don't write exec in v_cmp_f_i32
..

arch-gcn3,arch-vega: Don't write exec in v_cmp_f_i32

Per the GCN3 and VEGA ISAs, v_cmpx_* writes exec, while v_cmp_* doesn't.

This removes the erroneous exec write in the VOP3 implementation of
v_cmp_f_i32.

Change-Id: I048e35917163c45b879f38d31a88f3f3d56c0baf
---
M src/arch/amdgpu/vega/insts/instructions.cc
M src/arch/amdgpu/gcn3/insts/instructions.cc
2 files changed, 14 insertions(+), 2 deletions(-)



diff --git a/src/arch/amdgpu/gcn3/insts/instructions.cc  
b/src/arch/amdgpu/gcn3/insts/instructions.cc

index 65d008b..bb15957 100644
--- a/src/arch/amdgpu/gcn3/insts/instructions.cc
+++ b/src/arch/amdgpu/gcn3/insts/instructions.cc
@@ -20601,7 +20601,6 @@
 }
 }

-wf->execMask() = sdst.rawData();
 sdst.write();
 }

diff --git a/src/arch/amdgpu/vega/insts/instructions.cc  
b/src/arch/amdgpu/vega/insts/instructions.cc

index 757bfa8..1e07f0b 100644
--- a/src/arch/amdgpu/vega/insts/instructions.cc
+++ b/src/arch/amdgpu/vega/insts/instructions.cc
@@ -22454,7 +22454,6 @@
 }
 }

-wf->execMask() = sdst.rawData();
 sdst.write();
 } // execute
 // --- Inst_VOP3__V_CMP_LT_I32 class methods ---

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52445
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: I048e35917163c45b879f38d31a88f3f3d56c0baf
Gerrit-Change-Number: 52445
Gerrit-PatchSet: 1
Gerrit-Owner: Kyle Roarty 
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-cache: Ensure all fields of the CacheBlk class are initialized.

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


 (

1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.
 )Change subject: mem-cache: Ensure all fields of the CacheBlk class are  
initialized.

..

mem-cache: Ensure all fields of the CacheBlk class are initialized.

The constructor only initialized two fields, data and _tickInserted. The
print() method at least accesses the coherence status bits, which
valgrind determined were being accessed without being initialized.

This change adds a default initializer to all fields to prevent any
value from flapping around uninitialized.

Change-Id: Ie4c839504d49f9a131d8e3c3e8be02ff22f453a6
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52404
Reviewed-by: Jason Lowe-Power 
Reviewed-by: Daniel Carvalho 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M src/mem/cache/cache_blk.hh
1 file changed, 29 insertions(+), 8 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/mem/cache/cache_blk.hh b/src/mem/cache/cache_blk.hh
index b3eed9a..775efbe 100644
--- a/src/mem/cache/cache_blk.hh
+++ b/src/mem/cache/cache_blk.hh
@@ -100,13 +100,13 @@
  * data stored here should be kept consistant with the actual data
  * referenced by this block.
  */
-uint8_t *data;
+uint8_t *data = nullptr;

 /**
  * Which curTick() will this block be accessible. Its value is only
  * meaningful if the block is valid.
  */
-Tick whenReady;
+Tick whenReady = 0;

   protected:
 /**
@@ -152,7 +152,7 @@
 std::list lockList;

   public:
-CacheBlk() : TaggedEntry(), data(nullptr), _tickInserted(0)
+CacheBlk()
 {
 invalidate();
 }
@@ -474,22 +474,22 @@

   private:
 /** Task Id associated with this block */
-uint32_t _taskId;
+uint32_t _taskId = 0;

 /** holds the source requestor ID for this block. */
-int _srcRequestorId;
+int _srcRequestorId = 0;

 /** Number of references to this block since it was brought in. */
-unsigned _refCount;
+unsigned _refCount = 0;

 /**
  * Tick on which the block was inserted in the cache. Its value is only
  * meaningful if the block is valid.
  */
-Tick _tickInserted;
+Tick _tickInserted = 0;

 /** Whether this block is an unaccessed hardware prefetch. */
-bool _prefetched;
+bool _prefetched = 0;
 };

 /**

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52404
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: Ie4c839504d49f9a131d8e3c3e8be02ff22f453a6
Gerrit-Change-Number: 52404
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Nikos Nikoleris 
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]: stdlib: Fix resource downloader download to cwd upon failure

2021-11-04 Thread Bobby R. Bruce (Gerrit) via gem5-dev
Bobby R. Bruce has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/52423 )



Change subject: stdlib: Fix resource downloader download to cwd upon failure
..

stdlib: Fix resource downloader download to cwd upon failure

There are some cases where default downloading to `~/.cache/gem5` will
not work (for example, running gem5 in a Docker container, an error
observed here:
https://gem5-review.googlesource.com/c/public/gem5/+/51950).

To fix this, the `_get_default_resource_dir` has been altered to iterate
through a list of default resource directory targets. This change will
mean if `~/.cache/gem5` is not available then the resource is downloaded
to the current working directory of gem5.

Change-Id: I84e523f3adc182e140959243ff9335510d6b7185
---
M src/python/gem5/resources/resource.py
1 file changed, 42 insertions(+), 3 deletions(-)



diff --git a/src/python/gem5/resources/resource.py  
b/src/python/gem5/resources/resource.py

index b054b09..d3f1131 100644
--- a/src/python/gem5/resources/resource.py
+++ b/src/python/gem5/resources/resource.py
@@ -102,7 +102,7 @@
 :param resource_directory: The location of the directory in which  
the
 resource is to be stored. If this parameter is not set, it will  
set to
 the environment variable `GEM5_RESOURCE_DIR`. If the environment  
is not

-set it will default to `~/.cache/gem5`.
+set it will default to `~/.cache/gem5` if availbe, otherwise the  
CWD.

 :param override: If the resource is present, but does not have the
 correct md5 value, the resoruce will be deleted and re-downloaded  
if

 this value is True. Otherwise an exception will be thrown. False by
@@ -137,8 +137,28 @@

 def _get_default_resource_dir(cls) -> str:
 """
-Obtain the default gem5 resources directory on the host system.
+Obtain the default gem5 resources directory on the host system.  
This

+function will iterate through sensible targets until one that works
+on the host system.

 :returns: The default gem5 resources directory.
 """
-return os.path.join(Path.home(), ".cache", "gem5")
+test_list = [
+# First try `~/.cache/gem5`.
+os.path.join(Path.home(), ".cache", "gem5"),
+# Last resort, just put things in the cwd.
+os.path.join(Path.cwd(), "resources"),
+]
+
+for path in test_list:
+if os.path.exists(path): # If the path already exists...
+if os.path.isdir(path): # Check to see the path is a  
directory.

+return path # If so, the path is valid and can be used.
+else: # If the path does not exist, try to create it.
+try:
+os.makedirs(path, exist_ok=False)
+return path
+except OSError:
+continue # If the path cannot be created, then try  
another.

+
+raise Exception("Cannot find a valid location to download  
resources")


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52423
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: I84e523f3adc182e140959243ff9335510d6b7185
Gerrit-Change-Number: 52423
Gerrit-PatchSet: 1
Gerrit-Owner: Bobby R. Bruce 
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: Build failed in Jenkins: nightly #27

2021-11-04 Thread Jason Lowe-Power via gem5-dev
Glad you asked! I didn't look closely enough at the output.

Here's an error that looks suspicious (The whole file is attached.)

build/ARM_clang/cpu/o3/dyn_inst.hh:252:29: runtime error: constructor call
on misaligned address 0x1c1ed9ac for type 'gem5::PhysRegId *', which
requires 8 byte alignment
0x1c1ed9ac: note: pointer points here
  00 00 00 00 00 00 00 00  50 d9 1e 1c 00 00 00 00  70 8d 20 1c 00 00 00 00
 40 58 1e 1c 00 00 00 00
  ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
build/ARM_clang/cpu/o3/dyn_inst.hh:252:29 in
build/ARM_clang/cpu/o3/dyn_inst.hh:335:13: runtime error: store to
misaligned address 0x1c1ed9bc for type 'gem5::PhysRegIdPtr' (aka
'gem5::PhysRegId *'), which requires 8 byte alignment
0x1c1ed9bc: note: pointer points here
  70 8d 20 1c 00 00 00 00  40 58 1e 1c 00 00 00 00  1a 00 00 00 00 00 00 00
 1a 00 00 00 00 00 00 00
  ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
build/ARM_clang/cpu/o3/dyn_inst.hh:335:13 in
build/ARM_clang/cpu/o3/dyn_inst.hh:307:13: runtime error: store to
misaligned address 0x1c1ed9ac for type 'gem5::PhysRegIdPtr' (aka
'gem5::PhysRegId *'), which requires 8 byte alignment
0x1c1ed9ac: note: pointer points here
  00 00 00 00 00 00 00 00  50 d9 1e 1c 00 00 00 00  70 8d 20 1c 00 00 fb 1c
 00 00 00 00 80 10 fb 1c
  ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
build/ARM_clang/cpu/o3/dyn_inst.hh:307:13 in
build/ARM_clang/cpu/o3/dyn_inst.hh:322:13: runtime error: store to
misaligned address 0x1c1ed9b4 for type 'gem5::PhysRegIdPtr' (aka
'gem5::PhysRegId *'), which requires 8 byte alignment
0x1c1ed9b4: note: pointer points here
  00 00 00 00 00 00 00 00  70 8d 20 1c 00 00 fb 1c  00 00 00 00 80 10 fb 1c
 00 00 00 00 50 0d fb 1c
  ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
build/ARM_clang/cpu/o3/dyn_inst.hh:322:13 in
build/ARM_clang/cpu/o3/dyn_inst.hh:300:20: runtime error: load of
misaligned address 0x1c1ed9ac for type 'gem5::PhysRegIdPtr' (aka
'gem5::PhysRegId *'), which requires 8 byte alignment
0x1c1ed9ac: note: pointer points here
  00 00 00 00 f0 8f 01 18  00 00 00 00 c0 8c 01 18  00 00 00 00 00 00 fb 1c
 00 00 00 00 80 10 fb 1c
  ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
build/ARM_clang/cpu/o3/dyn_inst.hh:300:20 in
build/ARM_clang/cpu/o3/dyn_inst.hh:329:20: runtime error: load of
misaligned address 0x1d223754 for type 'gem5::PhysRegIdPtr' (aka
'gem5::PhysRegId *'), which requires 8 byte alignment
0x1d223754: note: pointer points here
  00 00 00 00 f0 8f 01 18  00 00 00 00 18 60 01 1d  00 00 00 00 78 60 01 1d
 00 00 00 00 78 60 01 1d
  ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
build/ARM_clang/cpu/o3/dyn_inst.hh:329:20 in
build/ARM_clang/cpu/o3/dyn_inst.hh:315:20: runtime error: load of
misaligned address 0x1c1ecba4 for type 'gem5::PhysRegIdPtr' (aka
'gem5::PhysRegId *'), which requires 8 byte alignment
0x1c1ecba4: note: pointer points here
  00 00 00 00 f8 90 01 18  00 00 00 00 f8 90 01 18  00 00 00 00 c0 60 01 1d
 00 00 00 00 78 60 01 1d
  ^
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
build/ARM_clang/cpu/o3/dyn_inst.hh:315:20 in


Cheers,
Jason

On Thu, Nov 4, 2021 at 2:51 PM Gabe Black  wrote:

> Did you find anything with ASAN?
>
> Gabe
>
> On Thu, Nov 4, 2021, 12:59 PM Gabe Black  wrote:
>
>> I don't know if they do, and frankly even the unique_ptr change could
>> since that could fix heap corruption, even though it's unlikely since I
>> don't think this regression uses any of that code (except maybe VIO). We
>> don't actually *know* that that change is at fault, even though I agree
>> that it seems like a reasonable guess. This could be one of those bugs
>> where something totally unrelated is wrong, and another change just shifts
>> things around until some address lines up with something bad.
>>
>> As valgrind grinds on, I think there's a good chance of identifying
>> exactly what the problem is. I think that should happen in the next day or
>> two, hopefully.
>>
>> Gabe
>>
>> On Thu, Nov 4, 2021 at 9:55 AM Jason Lowe-Power 
>> wrote:
>>
>>> Hey Gabe,
>>>
>>> Do these fix the nightly regression? If not, we may need to back out
>>> "4fe56ff72 - (3 months ago) arch-arm,cpu: Replace rename modes with split
>>> reg/elem register files." until we have a fix.
>>>
>>> Cheers,
>>> Jason
>>>
>>> On Thu, Nov 4, 2021 at 12:13 AM Gabe Black  wrote:
>>>
 Valgrind hasn't finished, but what it found so far is attached. I went
 through it and have the following changes which should address these
 uninitialized accesses, and an inefficiency in the cache it found by
 accident.

 https://gem5-review.googlesource.com/c/public/gem5/+/52403
 https://gem5-review.googlesource.com/c/public/gem5/+/52404
 https://gem5-review.googlesource.com/c/public/gem5/+/52405
 

[gem5-dev] Re: Build failed in Jenkins: nightly #27

2021-11-04 Thread Gabe Black via gem5-dev
Did you find anything with ASAN?

Gabe

On Thu, Nov 4, 2021, 12:59 PM Gabe Black  wrote:

> I don't know if they do, and frankly even the unique_ptr change could
> since that could fix heap corruption, even though it's unlikely since I
> don't think this regression uses any of that code (except maybe VIO). We
> don't actually *know* that that change is at fault, even though I agree
> that it seems like a reasonable guess. This could be one of those bugs
> where something totally unrelated is wrong, and another change just shifts
> things around until some address lines up with something bad.
>
> As valgrind grinds on, I think there's a good chance of identifying
> exactly what the problem is. I think that should happen in the next day or
> two, hopefully.
>
> Gabe
>
> On Thu, Nov 4, 2021 at 9:55 AM Jason Lowe-Power 
> wrote:
>
>> Hey Gabe,
>>
>> Do these fix the nightly regression? If not, we may need to back out
>> "4fe56ff72 - (3 months ago) arch-arm,cpu: Replace rename modes with split
>> reg/elem register files." until we have a fix.
>>
>> Cheers,
>> Jason
>>
>> On Thu, Nov 4, 2021 at 12:13 AM Gabe Black  wrote:
>>
>>> Valgrind hasn't finished, but what it found so far is attached. I went
>>> through it and have the following changes which should address these
>>> uninitialized accesses, and an inefficiency in the cache it found by
>>> accident.
>>>
>>> https://gem5-review.googlesource.com/c/public/gem5/+/52403
>>> https://gem5-review.googlesource.com/c/public/gem5/+/52404
>>> https://gem5-review.googlesource.com/c/public/gem5/+/52405
>>> https://gem5-review.googlesource.com/c/public/gem5/+/52406
>>>
>>> Gabe
>>>
>>> On Wed, Nov 3, 2021 at 12:45 PM Jason Lowe-Power 
>>> wrote:
>>>
 Here's my data:

 BAD * 4fe56ff72 - (3 months ago) arch-arm,cpu: Replace rename modes
 with split reg/elem register files. - Gabe Black
 GOOD * 25138cbb7 - (4 weeks ago) arch: Simplify and tidy up PCState
 classes. - Gabe Black
 * 930986332 - (7 days ago) mem: Fix whitespace in
 mem/ruby/system/Sequencer.py. - Gabe Black
 GOOD * 69e6ea485 - (3 months ago) arch-arm: Add walkBits method to
 PageTableOps - Giacomo Travaglini
 * 1268c6ec3 - (2 weeks ago) arch-arm: Expose LookupLevel enum to the
 python world - Giacomo Travaglini

 I've tested this a number of times and tested commits before and after
 these commits. I have increasing confidence (though no certainty) that
 4fe56ff72 is the culprit.

 I'm running a UBSAN now to see if that will help at all.

 Cheers,
 Jason

 On Tue, Nov 2, 2021 at 5:28 PM Gabe Black  wrote:

> I'm going to kill it now, and restart it with --track-origins=yes and
> in a separate tree so I can keep working on other things while it runs.
>
> Gabe
>
> On Tue, Nov 2, 2021 at 5:11 PM Gabe Black 
> wrote:
>
>> It's still running, but here's what it's found so far.
>>
>> Gabe
>>
>> On Tue, Nov 2, 2021 at 7:48 AM Jason Lowe-Power 
>> wrote:
>>
>>> Thanks! I tried a bisect but, tbh, it wasn't helpful since the error
>>> doesn't seem to be deterministic.
>>>
>>> As more evidence that it's a memory issue, the backtrace that I saw
>>> with GDB was something a bit different.
>>>
>>> Cheers,
>>> Jason
>>>
>>> On Tue, Nov 2, 2021 at 5:07 AM Gabe Black 
>>> wrote:
>>>
 I'm running it under valgrind to see if that tells me anything,
 which is going to take a while. I'll let you know if/when it finishes.

 Gabe

 On Tue, Nov 2, 2021 at 4:36 AM Gabe Black 
 wrote:

> Attached is a log of a failing run, and backtrace of the segfault.
>
> Gabe
>
> On Tue, Nov 2, 2021 at 4:17 AM Gabe Black 
> wrote:
>
>> Ok, I reproduced the segfault once, but then running again in gdb
>> it exited normally. I'm pretty confident it's something to do with 
>> things
>> getting cleaned up and/or destructed at the end of the simulation, 
>> but
>> until I catch something in the act of exploding I won't be able to 
>> nail
>> down specifically what's doing it.
>>
>> Gabe
>>
>> On Tue, Nov 2, 2021 at 3:46 AM Gabe Black 
>> wrote:
>>
>>> A clean build seems to have fixed the IdeDisk problem.
>>>
>>> On Tue, Nov 2, 2021 at 3:35 AM Gabe Black 
>>> wrote:
>>>
 Can you get a backtrace from it? Or run it in valgrind? I'm
 trying to use the command line you provided locally, but it's 
 complaining
 about not being able to find IdeDisk which is very strange... I 
 don't think
 I have an account on the machine where this ran, but ideally I'll 
 be able
 to reproduce it locally where it 

[gem5-dev] default m5ops_base to off on x86

2021-11-04 Thread Gabe Black via gem5-dev
Hey folks. One of the last things to sort out before I'd consider the gem5
mutli-ISA support at least usable is that the m5ops_base config parameter
defaults to zero, except if you're on x86 where it defaults to 0x.

I think it would make sense to instead default it to 0 all the time,
effectively disabled, and to expect a config to enable it even on x86 to
something that makes sense on that platform. Even though 0x may be
a typical address to use for x86, even there it might not be appropriate,
since that's where the BIOS would be if doing a bare metal boot. I think if
it's reasonable to expect other ISAs to have to set this address, x86
should be able to manage as well. There is already special handling in
se.py for the x86/kvm/se mode combination, so there is an easy place to
slot this setting in to at least that config.

I think this change makes sense, but before I changed behavior like that
(as opposed to just changing implementation), I wanted to check to see if
there are any strong opinions out there, or other suggestions for how to
handle this.

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: Build failed in Jenkins: nightly #27

2021-11-04 Thread Gabe Black via gem5-dev
I don't know if they do, and frankly even the unique_ptr change could since
that could fix heap corruption, even though it's unlikely since I don't
think this regression uses any of that code (except maybe VIO). We don't
actually *know* that that change is at fault, even though I agree that it
seems like a reasonable guess. This could be one of those bugs where
something totally unrelated is wrong, and another change just shifts things
around until some address lines up with something bad.

As valgrind grinds on, I think there's a good chance of identifying exactly
what the problem is. I think that should happen in the next day or two,
hopefully.

Gabe

On Thu, Nov 4, 2021 at 9:55 AM Jason Lowe-Power  wrote:

> Hey Gabe,
>
> Do these fix the nightly regression? If not, we may need to back out
> "4fe56ff72 - (3 months ago) arch-arm,cpu: Replace rename modes with split
> reg/elem register files." until we have a fix.
>
> Cheers,
> Jason
>
> On Thu, Nov 4, 2021 at 12:13 AM Gabe Black  wrote:
>
>> Valgrind hasn't finished, but what it found so far is attached. I went
>> through it and have the following changes which should address these
>> uninitialized accesses, and an inefficiency in the cache it found by
>> accident.
>>
>> https://gem5-review.googlesource.com/c/public/gem5/+/52403
>> https://gem5-review.googlesource.com/c/public/gem5/+/52404
>> https://gem5-review.googlesource.com/c/public/gem5/+/52405
>> https://gem5-review.googlesource.com/c/public/gem5/+/52406
>>
>> Gabe
>>
>> On Wed, Nov 3, 2021 at 12:45 PM Jason Lowe-Power 
>> wrote:
>>
>>> Here's my data:
>>>
>>> BAD * 4fe56ff72 - (3 months ago) arch-arm,cpu: Replace rename modes with
>>> split reg/elem register files. - Gabe Black
>>> GOOD * 25138cbb7 - (4 weeks ago) arch: Simplify and tidy up PCState
>>> classes. - Gabe Black
>>> * 930986332 - (7 days ago) mem: Fix whitespace in
>>> mem/ruby/system/Sequencer.py. - Gabe Black
>>> GOOD * 69e6ea485 - (3 months ago) arch-arm: Add walkBits method to
>>> PageTableOps - Giacomo Travaglini
>>> * 1268c6ec3 - (2 weeks ago) arch-arm: Expose LookupLevel enum to the
>>> python world - Giacomo Travaglini
>>>
>>> I've tested this a number of times and tested commits before and after
>>> these commits. I have increasing confidence (though no certainty) that
>>> 4fe56ff72 is the culprit.
>>>
>>> I'm running a UBSAN now to see if that will help at all.
>>>
>>> Cheers,
>>> Jason
>>>
>>> On Tue, Nov 2, 2021 at 5:28 PM Gabe Black  wrote:
>>>
 I'm going to kill it now, and restart it with --track-origins=yes and
 in a separate tree so I can keep working on other things while it runs.

 Gabe

 On Tue, Nov 2, 2021 at 5:11 PM Gabe Black  wrote:

> It's still running, but here's what it's found so far.
>
> Gabe
>
> On Tue, Nov 2, 2021 at 7:48 AM Jason Lowe-Power 
> wrote:
>
>> Thanks! I tried a bisect but, tbh, it wasn't helpful since the error
>> doesn't seem to be deterministic.
>>
>> As more evidence that it's a memory issue, the backtrace that I saw
>> with GDB was something a bit different.
>>
>> Cheers,
>> Jason
>>
>> On Tue, Nov 2, 2021 at 5:07 AM Gabe Black 
>> wrote:
>>
>>> I'm running it under valgrind to see if that tells me anything,
>>> which is going to take a while. I'll let you know if/when it finishes.
>>>
>>> Gabe
>>>
>>> On Tue, Nov 2, 2021 at 4:36 AM Gabe Black 
>>> wrote:
>>>
 Attached is a log of a failing run, and backtrace of the segfault.

 Gabe

 On Tue, Nov 2, 2021 at 4:17 AM Gabe Black 
 wrote:

> Ok, I reproduced the segfault once, but then running again in gdb
> it exited normally. I'm pretty confident it's something to do with 
> things
> getting cleaned up and/or destructed at the end of the simulation, but
> until I catch something in the act of exploding I won't be able to 
> nail
> down specifically what's doing it.
>
> Gabe
>
> On Tue, Nov 2, 2021 at 3:46 AM Gabe Black 
> wrote:
>
>> A clean build seems to have fixed the IdeDisk problem.
>>
>> On Tue, Nov 2, 2021 at 3:35 AM Gabe Black 
>> wrote:
>>
>>> Can you get a backtrace from it? Or run it in valgrind? I'm
>>> trying to use the command line you provided locally, but it's 
>>> complaining
>>> about not being able to find IdeDisk which is very strange... I 
>>> don't think
>>> I have an account on the machine where this ran, but ideally I'll 
>>> be able
>>> to reproduce it locally where it will be easier to work with.
>>>
>>> Gabe
>>>
>>> On Mon, Nov 1, 2021 at 3:03 PM Jason Lowe-Power <
>>> ja...@lowepower.com> wrote:
>>>
 After spending some time on this, there is definitely a

[gem5-dev] Re: preview branch for mutli-ISA gem5?

2021-11-04 Thread Gabe Black via gem5-dev
To be clear, I'm not suggesting a branch that gets merged with develop at
some intersection point, I'm just suggesting a branch where the changes are
all stacked together so they're easy to try out. There are too many
interactions between these changes and the rest of gem5 to expect there not
to be disaster when trying to merge things if they've had time to stew
apart from each other. Trust me, it's been a pain to keep things working as
I've been rebasing them over time.

I just want people to be able to see what the end goal is, and that it's
working. Also there are people who are very interested in this capability,
and I want to make it easy for them to use it without having to wait for a
few months for the reviews to trickle through.

Gabe

On Thu, Nov 4, 2021 at 11:41 AM Bobby Bruce  wrote:

> I think it's a great idea and we should probably do it more often for this
> big long changes we don't want to straddle a release.
>
> --
> Dr. Bobby R. Bruce
> Room 3050,
> Kemper Hall, UC Davis
> Davis,
> CA, 95616
>
> web: https://www.bobbybruce.net
>
>
> On Thu, Nov 4, 2021 at 7:42 AM Jason Lowe-Power via gem5-dev <
> gem5-dev@gem5.org> wrote:
>
>> Seems like a good idea to me! I like the idea of having more feature
>> branches for long-lived relation chains.
>>
>> Given the timing, it seems like it would be a good idea to hold off
>> integrating this until gem5 22.0. If we have a branch we can test for a
>> while, it will be less likely to break our users use cases.
>>
>> Cheers,
>> Jason
>>
>> On Wed, Nov 3, 2021 at 11:51 PM Gabe Black via gem5-dev <
>> gem5-dev@gem5.org> wrote:
>>
>>> Hey folks, I'm not quite ready for it yet, but I'm getting close to
>>> having everything at least written for a multi-ISA version of gem5. What do
>>> you think about having a preview branch on gerrit? I can create one pretty
>>> easily, I just want to make sure that's appropriate and something other
>>> people might be interested in before I do so. Like I said, I haven't gotten
>>> *quite* to the point where I have something useable, but I think I'm down
>>> to the last few things.
>>>
>>> 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 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]: mem-cache: Don't generate debug output unless you're going to use it.

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


 (

1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.
 )Change subject: mem-cache: Don't generate debug output unless you're  
going to use it.

..

mem-cache: Don't generate debug output unless you're going to use it.

The BaseCache::handleFill function would generate an "old_state" string
unconditionally, just in case it would need to print it out later on in
the function if the Cache debug variable was set. This is very wasteful.
We should only generate that string if we are actually going to use it
later on.

Change-Id: I4a570d1cd2814e5a089eac1233dedd1801d68975
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52405
Reviewed-by: Jason Lowe-Power 
Reviewed-by: Daniel Carvalho 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M src/mem/cache/base.cc
1 file changed, 21 insertions(+), 1 deletion(-)

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/mem/cache/base.cc b/src/mem/cache/base.cc
index fa9257c..4007a4f 100644
--- a/src/mem/cache/base.cc
+++ b/src/mem/cache/base.cc
@@ -1439,7 +1439,7 @@
 Addr addr = pkt->getAddr();
 bool is_secure = pkt->isSecure();
 const bool has_old_data = blk && blk->isValid();
-const std::string old_state = blk ? blk->print() : "";
+const std::string old_state = (debug::Cache && blk) ?  
blk->print() : "";


 // When handling a fill, we should have no writes to this line.
 assert(addr == pkt->getBlockAddr(blkSize));

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52405
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: I4a570d1cd2814e5a089eac1233dedd1801d68975
Gerrit-Change-Number: 52405
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Nikos Nikoleris 
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]: misc: Add 'stdlib' tag to MAINTAINERS.yaml

2021-11-04 Thread Bobby R. Bruce (Gerrit) via gem5-dev
Bobby R. Bruce has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/51791 )


Change subject: misc: Add 'stdlib' tag to MAINTAINERS.yaml
..

misc: Add 'stdlib' tag to MAINTAINERS.yaml

This tag is for the "gem5 standard library" which can be found in
`src/python/gem5`.

Change-Id: Idf276635cd3f1d729cfbb4b7195b20fb6584
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/51791
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M MAINTAINERS.yaml
1 file changed, 23 insertions(+), 0 deletions(-)

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




diff --git a/MAINTAINERS.yaml b/MAINTAINERS.yaml
index 7868313..c8b8957 100644
--- a/MAINTAINERS.yaml
+++ b/MAINTAINERS.yaml
@@ -201,6 +201,13 @@
   maintainers:
 - Jason Lowe-Power 

+stdlib:
+  desc: >-
+The gem5 standard library found under `src/python/gem5`
+  status: maintained
+  maintainers:
+- Bobby R. Bruce 
+
 mem:
   desc: >-
 General memory system (e.g., XBar, Packet)

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/51791
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: Idf276635cd3f1d729cfbb4b7195b20fb6584
Gerrit-Change-Number: 51791
Gerrit-PatchSet: 6
Gerrit-Owner: Bobby R. Bruce 
Gerrit-Reviewer: Bobby R. 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] Re: preview branch for mutli-ISA gem5?

2021-11-04 Thread Bobby Bruce via gem5-dev
I think it's a great idea and we should probably do it more often for this
big long changes we don't want to straddle a release.

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

web: https://www.bobbybruce.net


On Thu, Nov 4, 2021 at 7:42 AM Jason Lowe-Power via gem5-dev <
gem5-dev@gem5.org> wrote:

> Seems like a good idea to me! I like the idea of having more feature
> branches for long-lived relation chains.
>
> Given the timing, it seems like it would be a good idea to hold off
> integrating this until gem5 22.0. If we have a branch we can test for a
> while, it will be less likely to break our users use cases.
>
> Cheers,
> Jason
>
> On Wed, Nov 3, 2021 at 11:51 PM Gabe Black via gem5-dev 
> wrote:
>
>> Hey folks, I'm not quite ready for it yet, but I'm getting close to
>> having everything at least written for a multi-ISA version of gem5. What do
>> you think about having a preview branch on gerrit? I can create one pretty
>> easily, I just want to make sure that's appropriate and something other
>> people might be interested in before I do so. Like I said, I haven't gotten
>> *quite* to the point where I have something useable, but I think I'm down
>> to the last few things.
>>
>> 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 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] Re: Build failed in Jenkins: nightly #27

2021-11-04 Thread Jason Lowe-Power via gem5-dev
Hey Gabe,

Do these fix the nightly regression? If not, we may need to back out
"4fe56ff72 - (3 months ago) arch-arm,cpu: Replace rename modes with split
reg/elem register files." until we have a fix.

Cheers,
Jason

On Thu, Nov 4, 2021 at 12:13 AM Gabe Black  wrote:

> Valgrind hasn't finished, but what it found so far is attached. I went
> through it and have the following changes which should address these
> uninitialized accesses, and an inefficiency in the cache it found by
> accident.
>
> https://gem5-review.googlesource.com/c/public/gem5/+/52403
> https://gem5-review.googlesource.com/c/public/gem5/+/52404
> https://gem5-review.googlesource.com/c/public/gem5/+/52405
> https://gem5-review.googlesource.com/c/public/gem5/+/52406
>
> Gabe
>
> On Wed, Nov 3, 2021 at 12:45 PM Jason Lowe-Power 
> wrote:
>
>> Here's my data:
>>
>> BAD * 4fe56ff72 - (3 months ago) arch-arm,cpu: Replace rename modes with
>> split reg/elem register files. - Gabe Black
>> GOOD * 25138cbb7 - (4 weeks ago) arch: Simplify and tidy up PCState
>> classes. - Gabe Black
>> * 930986332 - (7 days ago) mem: Fix whitespace in
>> mem/ruby/system/Sequencer.py. - Gabe Black
>> GOOD * 69e6ea485 - (3 months ago) arch-arm: Add walkBits method to
>> PageTableOps - Giacomo Travaglini
>> * 1268c6ec3 - (2 weeks ago) arch-arm: Expose LookupLevel enum to the
>> python world - Giacomo Travaglini
>>
>> I've tested this a number of times and tested commits before and after
>> these commits. I have increasing confidence (though no certainty) that
>> 4fe56ff72 is the culprit.
>>
>> I'm running a UBSAN now to see if that will help at all.
>>
>> Cheers,
>> Jason
>>
>> On Tue, Nov 2, 2021 at 5:28 PM Gabe Black  wrote:
>>
>>> I'm going to kill it now, and restart it with --track-origins=yes and in
>>> a separate tree so I can keep working on other things while it runs.
>>>
>>> Gabe
>>>
>>> On Tue, Nov 2, 2021 at 5:11 PM Gabe Black  wrote:
>>>
 It's still running, but here's what it's found so far.

 Gabe

 On Tue, Nov 2, 2021 at 7:48 AM Jason Lowe-Power 
 wrote:

> Thanks! I tried a bisect but, tbh, it wasn't helpful since the error
> doesn't seem to be deterministic.
>
> As more evidence that it's a memory issue, the backtrace that I saw
> with GDB was something a bit different.
>
> Cheers,
> Jason
>
> On Tue, Nov 2, 2021 at 5:07 AM Gabe Black 
> wrote:
>
>> I'm running it under valgrind to see if that tells me anything, which
>> is going to take a while. I'll let you know if/when it finishes.
>>
>> Gabe
>>
>> On Tue, Nov 2, 2021 at 4:36 AM Gabe Black 
>> wrote:
>>
>>> Attached is a log of a failing run, and backtrace of the segfault.
>>>
>>> Gabe
>>>
>>> On Tue, Nov 2, 2021 at 4:17 AM Gabe Black 
>>> wrote:
>>>
 Ok, I reproduced the segfault once, but then running again in gdb
 it exited normally. I'm pretty confident it's something to do with 
 things
 getting cleaned up and/or destructed at the end of the simulation, but
 until I catch something in the act of exploding I won't be able to nail
 down specifically what's doing it.

 Gabe

 On Tue, Nov 2, 2021 at 3:46 AM Gabe Black 
 wrote:

> A clean build seems to have fixed the IdeDisk problem.
>
> On Tue, Nov 2, 2021 at 3:35 AM Gabe Black 
> wrote:
>
>> Can you get a backtrace from it? Or run it in valgrind? I'm
>> trying to use the command line you provided locally, but it's 
>> complaining
>> about not being able to find IdeDisk which is very strange... I 
>> don't think
>> I have an account on the machine where this ran, but ideally I'll be 
>> able
>> to reproduce it locally where it will be easier to work with.
>>
>> Gabe
>>
>> On Mon, Nov 1, 2021 at 3:03 PM Jason Lowe-Power <
>> ja...@lowepower.com> wrote:
>>
>>> After spending some time on this, there is definitely a segfault
>>> at the end of execution. It's odd that the testing scripts sometimes
>>> reports that it works.
>>>
>>> If you run the following, you should see a segfault at the end
>>> and no stats are generated:
>>> ../gem5/> build/ARM/gem5.opt tests/gem5/fs/linux/arm/run.py
>>> tests/gem5/configs/realview-o3.py tests/gem5/resources/arm .
>>>
>>> I also notice that when it doesn't fail the following is printed:
>>> "build/ARM/arch/arm/isa.hh:650: warn: User mode does not have
>>> SPSR
>>> build/ARM/arch/arm/isa.hh:650: warn: User mode does not have
>>> SPSR"
>>>
>>> But it's not printed when it fails.
>>>
>>> Cheers,
>>> Jason
>>>
>>> On Mon, Nov 1, 2021 at 8:29 AM Jason Lowe-Power <

[gem5-dev] Re: preview branch for mutli-ISA gem5?

2021-11-04 Thread Jason Lowe-Power via gem5-dev
Seems like a good idea to me! I like the idea of having more feature
branches for long-lived relation chains.

Given the timing, it seems like it would be a good idea to hold off
integrating this until gem5 22.0. If we have a branch we can test for a
while, it will be less likely to break our users use cases.

Cheers,
Jason

On Wed, Nov 3, 2021 at 11:51 PM Gabe Black via gem5-dev 
wrote:

> Hey folks, I'm not quite ready for it yet, but I'm getting close to having
> everything at least written for a multi-ISA version of gem5. What do you
> think about having a preview branch on gerrit? I can create one pretty
> easily, I just want to make sure that's appropriate and something other
> people might be interested in before I do so. Like I said, I haven't gotten
> *quite* to the point where I have something useable, but I think I'm down
> to the last few things.
>
> 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 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: nightly #32

2021-11-04 Thread jenkins-no-reply--- via gem5-dev
See 

Changes:

[giacomo.travaglini] arch: Fix serialization/deserialization of Vector registers

[Bobby R. Bruce] python: Remove incorrect usage of typing 'Optional'

[Bobby R. Bruce] tests: Removing Atomic CPU with Ruby tests

[gabe.black] cpu-kvm,arch-x86,arch-arm,dev: Pair operator new with operator 
delete.


--
[...truncated 840.71 KB...]
[--] 4 tests from CheckpointInFixture
[ RUN  ] CheckpointInFixture.FindSections
[   OK ] CheckpointInFixture.FindSections (0 ms)
[ RUN  ] CheckpointInFixture.FindEntries
[   OK ] CheckpointInFixture.FindEntries (0 ms)
[ RUN  ] CheckpointInFixture.ExtractEntries
[   OK ] CheckpointInFixture.ExtractEntries (0 ms)
[ RUN  ] CheckpointInFixture.SCSCptInPathScoped
[   OK ] CheckpointInFixture.SCSCptInPathScoped (1 ms)
[--] 4 tests from CheckpointInFixture (1 ms total)

[--] 6 tests from SerializableFixture
[ RUN  ] SerializableFixture.SCSChangeCptOutSingle
[   OK ] SerializableFixture.SCSChangeCptOutSingle (0 ms)
[ RUN  ] SerializableFixture.SCSChangeCptOutMultiple
[   OK ] SerializableFixture.SCSChangeCptOutMultiple (0 ms)
[ RUN  ] SerializableFixture.SCSChangeCptOutLarge
[   OK ] SerializableFixture.SCSChangeCptOutLarge (0 ms)
[ RUN  ] SerializableFixture.SectionSerializationSimple
[   OK ] SerializableFixture.SectionSerializationSimple (0 ms)
[ RUN  ] SerializableFixture.ParamOutIn
[   OK ] SerializableFixture.ParamOutIn (0 ms)
[ RUN  ] SerializableFixture.ParamOutInMultipleSections
[   OK ] SerializableFixture.ParamOutInMultipleSections (0 ms)
[--] 6 tests from SerializableFixture (1 ms total)

[--] Global test environment tear-down
[==] 33 tests from 8 test suites ran. (157 ms total)
[  PASSED  ] 33 tests.

  YOU HAVE 1 DISABLED TEST

[   OK ] LoggingDeathTest.PanicLoggerExitHelper (167 ms)
[ RUN  ] LoggingDeathTest.ExitMessage
[   OK ] LoggingDeathTest.ExitMessage (150 ms)
[ RUN  ] LoggingDeathTest.Panic
[   OK ] LoggingDeathTest.Panic (151 ms)
[ RUN  ] LoggingDeathTest.Fatal
[   OK ] LoggingDeathTest.Fatal (3 ms)
[ RUN  ] LoggingDeathTest.PanicIf
[   OK ] LoggingDeathTest.PanicIf (150 ms)
[ RUN  ] LoggingDeathTest.FatalIf
[   OK ] LoggingDeathTest.FatalIf (3 ms)
[ RUN  ] LoggingDeathTest.gem5Assert
[  SKIPPED ] LoggingDeathTest.gem5Assert (0 ms)
[--] 13 tests from LoggingDeathTest (1228 ms total)

[--] 21 tests from LoggingFixture
[ RUN  ] LoggingFixture.BasicPrint
[   OK ] LoggingFixture.BasicPrint (0 ms)
[ RUN  ] LoggingFixture.VariadicCharPrint
[   OK ] LoggingFixture.VariadicCharPrint (0 ms)
[ RUN  ] LoggingFixture.VariadicStringPrint
[   OK ] LoggingFixture.VariadicStringPrint (0 ms)
[ RUN  ] LoggingFixture.VariadicCharMissingPrint
[   OK ] LoggingFixture.VariadicCharMissingPrint (0 ms)
[ RUN  ] LoggingFixture.VariadicStringMissingPrint
[   OK ] LoggingFixture.VariadicStringMissingPrint (0 ms)
[ RUN  ] LoggingFixture.DisabledPrint
[   OK ] LoggingFixture.DisabledPrint (0 ms)
[ RUN  ] LoggingFixture.WarnLoggerPrint
[   OK ] LoggingFixture.WarnLoggerPrint (0 ms)
[ RUN  ] LoggingFixture.InfoLoggerPrint
[   OK ] LoggingFixture.InfoLoggerPrint (0 ms)
[ RUN  ] LoggingFixture.HackLoggerPrint
[   OK ] LoggingFixture.HackLoggerPrint (0 ms)
[ RUN  ] LoggingFixture.FatalLoggerPrint
[   OK ] LoggingFixture.FatalLoggerPrint (0 ms)
[ RUN  ] LoggingFixture.PanicLoggerPrint
[   OK ] LoggingFixture.PanicLoggerPrint (0 ms)
[ RUN  ] LoggingFixture.BaseMessage
[   OK ] LoggingFixture.BaseMessage (0 ms)
[ RUN  ] LoggingFixture.BaseMessageOnce
[   OK ] LoggingFixture.BaseMessageOnce (0 ms)
[ RUN  ] LoggingFixture.Warn
[   OK ] LoggingFixture.Warn (0 ms)
[ RUN  ] LoggingFixture.Inform
[   OK ] LoggingFixture.Inform (0 ms)
[ RUN  ] LoggingFixture.Hack
[   OK ] LoggingFixture.Hack (0 ms)
[ RUN  ] LoggingFixture.WarnOnce
[   OK ] LoggingFixture.WarnOnce (0 ms)
[ RUN  ] LoggingFixture.InformOnce
[   OK ] LoggingFixture.InformOnce (0 ms)
[ RUN  ] LoggingFixture.HackOnce
[   OK ] LoggingFixture.HackOnce (0 ms)
[ RUN  ] LoggingFixture.WarnIf
[   OK ] LoggingFixture.WarnIf (0 ms)
[ RUN  ] LoggingFixture.WarnIfOnce
[   OK ] LoggingFixture.WarnIfOnce (0 ms)
[--] 21 tests from LoggingFixture (0 ms total)

[--] Global test environment tear-down
[==] 34 tests from 2 test suites ran. (1228 ms total)
[  PASSED  ] 32 tests.
[  SKIPPED ] 2 tests, listed below:
[  SKIPPED ] LoggingDeathTest.EmptyPrefix
[  SKIPPED ] LoggingDeathTest.gem5Assert
scons: done building targets.
*** Summary of Warnings ***
Warning: Deprecated namespaces are not supported by this compiler.
 Please make sure to check the mailing 

[gem5-dev] Re: Build failed in Jenkins: nightly #27

2021-11-04 Thread Gabe Black via gem5-dev
Valgrind hasn't finished, but what it found so far is attached. I went
through it and have the following changes which should address these
uninitialized accesses, and an inefficiency in the cache it found by
accident.

https://gem5-review.googlesource.com/c/public/gem5/+/52403
https://gem5-review.googlesource.com/c/public/gem5/+/52404
https://gem5-review.googlesource.com/c/public/gem5/+/52405
https://gem5-review.googlesource.com/c/public/gem5/+/52406

Gabe

On Wed, Nov 3, 2021 at 12:45 PM Jason Lowe-Power 
wrote:

> Here's my data:
>
> BAD * 4fe56ff72 - (3 months ago) arch-arm,cpu: Replace rename modes with
> split reg/elem register files. - Gabe Black
> GOOD * 25138cbb7 - (4 weeks ago) arch: Simplify and tidy up PCState
> classes. - Gabe Black
> * 930986332 - (7 days ago) mem: Fix whitespace in
> mem/ruby/system/Sequencer.py. - Gabe Black
> GOOD * 69e6ea485 - (3 months ago) arch-arm: Add walkBits method to
> PageTableOps - Giacomo Travaglini
> * 1268c6ec3 - (2 weeks ago) arch-arm: Expose LookupLevel enum to the
> python world - Giacomo Travaglini
>
> I've tested this a number of times and tested commits before and after
> these commits. I have increasing confidence (though no certainty) that
> 4fe56ff72 is the culprit.
>
> I'm running a UBSAN now to see if that will help at all.
>
> Cheers,
> Jason
>
> On Tue, Nov 2, 2021 at 5:28 PM Gabe Black  wrote:
>
>> I'm going to kill it now, and restart it with --track-origins=yes and in
>> a separate tree so I can keep working on other things while it runs.
>>
>> Gabe
>>
>> On Tue, Nov 2, 2021 at 5:11 PM Gabe Black  wrote:
>>
>>> It's still running, but here's what it's found so far.
>>>
>>> Gabe
>>>
>>> On Tue, Nov 2, 2021 at 7:48 AM Jason Lowe-Power 
>>> wrote:
>>>
 Thanks! I tried a bisect but, tbh, it wasn't helpful since the error
 doesn't seem to be deterministic.

 As more evidence that it's a memory issue, the backtrace that I saw
 with GDB was something a bit different.

 Cheers,
 Jason

 On Tue, Nov 2, 2021 at 5:07 AM Gabe Black  wrote:

> I'm running it under valgrind to see if that tells me anything, which
> is going to take a while. I'll let you know if/when it finishes.
>
> Gabe
>
> On Tue, Nov 2, 2021 at 4:36 AM Gabe Black 
> wrote:
>
>> Attached is a log of a failing run, and backtrace of the segfault.
>>
>> Gabe
>>
>> On Tue, Nov 2, 2021 at 4:17 AM Gabe Black 
>> wrote:
>>
>>> Ok, I reproduced the segfault once, but then running again in gdb it
>>> exited normally. I'm pretty confident it's something to do with things
>>> getting cleaned up and/or destructed at the end of the simulation, but
>>> until I catch something in the act of exploding I won't be able to nail
>>> down specifically what's doing it.
>>>
>>> Gabe
>>>
>>> On Tue, Nov 2, 2021 at 3:46 AM Gabe Black 
>>> wrote:
>>>
 A clean build seems to have fixed the IdeDisk problem.

 On Tue, Nov 2, 2021 at 3:35 AM Gabe Black 
 wrote:

> Can you get a backtrace from it? Or run it in valgrind? I'm trying
> to use the command line you provided locally, but it's complaining 
> about
> not being able to find IdeDisk which is very strange... I don't think 
> I
> have an account on the machine where this ran, but ideally I'll be 
> able to
> reproduce it locally where it will be easier to work with.
>
> Gabe
>
> On Mon, Nov 1, 2021 at 3:03 PM Jason Lowe-Power <
> ja...@lowepower.com> wrote:
>
>> After spending some time on this, there is definitely a segfault
>> at the end of execution. It's odd that the testing scripts sometimes
>> reports that it works.
>>
>> If you run the following, you should see a segfault at the end
>> and no stats are generated:
>> ../gem5/> build/ARM/gem5.opt tests/gem5/fs/linux/arm/run.py
>> tests/gem5/configs/realview-o3.py tests/gem5/resources/arm .
>>
>> I also notice that when it doesn't fail the following is printed:
>> "build/ARM/arch/arm/isa.hh:650: warn: User mode does not have SPSR
>> build/ARM/arch/arm/isa.hh:650: warn: User mode does not have SPSR"
>>
>> But it's not printed when it fails.
>>
>> Cheers,
>> Jason
>>
>> On Mon, Nov 1, 2021 at 8:29 AM Jason Lowe-Power <
>> ja...@lowepower.com> wrote:
>>
>>> I don't think so. The binaries haven't been updated since April
>>> ('aarch-system-20210904.tar.bz2'). Well, the blame says April even 
>>> if the
>>> filename is confusing (DDMM?).
>>>
>>> Here's the failure:
>>> 

[gem5-dev] Change in gem5/gem5[develop]: dev-arm: Ensure all fields of GicV2 are initialized.

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



Change subject: dev-arm: Ensure all fields of GicV2 are initialized.
..

dev-arm: Ensure all fields of GicV2 are initialized.

The constructor tried to initialize all values, but in particular missed
intGroup, and may have missed other values as well.

This change adds default initializers to all member variables, and
removes the initializer in the constructor when that version is
redundant. This will ensure that all values are initialized, and make it
obvious when any aren't.

There are some arrays which are initialized by the constructor in a loop
in its body which will now be initialized twice, once with a default
value and once with the loop value. Since this only happens when the
SimObject is constructed and that only happens once during a simulation,
that is a trivial cost to pay for the added safety of knowing the data
structure is initialized.

Change-Id: Ibcd610e40259e46e3cde9b76c7f9ddc816832dfd
---
M src/dev/arm/gic_v2.cc
M src/dev/arm/gic_v2.hh
2 files changed, 49 insertions(+), 33 deletions(-)



diff --git a/src/dev/arm/gic_v2.cc b/src/dev/arm/gic_v2.cc
index bd780ee..bdf7e95 100644
--- a/src/dev/arm/gic_v2.cc
+++ b/src/dev/arm/gic_v2.cc
@@ -74,14 +74,8 @@
   addrRanges{distRange, cpuRange},
   distPioDelay(p.dist_pio_delay),
   cpuPioDelay(p.cpu_pio_delay), intLatency(p.int_latency),
-  enabled(false), haveGem5Extensions(p.gem5_extensions),
-  itLines(p.it_lines),
-  intEnabled {}, pendingInt {}, activeInt {},
-  intPriority {}, intConfig {}, cpuTarget {},
-  cpuSgiPending {}, cpuSgiActive {},
-  cpuSgiPendingExt {}, cpuSgiActiveExt {},
-  cpuPpiPending {}, cpuPpiActive {},
-  pendingDelayedInterrupts(0)
+  haveGem5Extensions(p.gem5_extensions),
+  itLines(p.it_lines)
 {
 for (int x = 0; x < CPU_MAX; x++) {
 iccrpr[x] = 0xff;
@@ -99,8 +93,6 @@
 }
 DPRINTF(Interrupt, "cpuEnabled[0]=%d cpuEnabled[1]=%d\n",  
cpuEnabled(0),

 cpuEnabled(1));
-
-gem5ExtensionsEnabled = false;
 }

 GicV2::~GicV2()
diff --git a/src/dev/arm/gic_v2.hh b/src/dev/arm/gic_v2.hh
index 9e68745..2233622 100644
--- a/src/dev/arm/gic_v2.hh
+++ b/src/dev/arm/gic_v2.hh
@@ -168,13 +168,13 @@

   protected:
 /** Gic enabled */
-bool enabled;
+bool enabled = false;

 /** Are gem5 extensions available? */
 const bool haveGem5Extensions;

 /** gem5 many-core extension enabled by driver */
-bool gem5ExtensionsEnabled;
+bool gem5ExtensionsEnabled = false;

 /** Number of itLines enabled */
 uint32_t itLines;
@@ -221,7 +221,7 @@
 /** GICD_I{S,C}ENABLER{1..31}
  * interrupt enable bits for global interrupts
  * 1b per interrupt, 32 bits per word, 31 words */
-uint32_t intEnabled[INT_BITS_MAX-1];
+uint32_t intEnabled[INT_BITS_MAX - 1] = {};

 uint32_t&
 getIntEnabled(ContextID ctx, uint32_t ix)
@@ -236,7 +236,7 @@
 /** GICD_I{S,C}PENDR{1..31}
  * interrupt pending bits for global interrupts
  * 1b per interrupt, 32 bits per word, 31 words */
-uint32_t pendingInt[INT_BITS_MAX-1];
+uint32_t pendingInt[INT_BITS_MAX - 1] = {};

 uint32_t&
 getPendingInt(ContextID ctx, uint32_t ix)
@@ -252,7 +252,7 @@
 /** GICD_I{S,C}ACTIVER{1..31}
  * interrupt active bits for global interrupts
  * 1b per interrupt, 32 bits per word, 31 words */
-uint32_t activeInt[INT_BITS_MAX-1];
+uint32_t activeInt[INT_BITS_MAX - 1] = {};

 uint32_t&
 getActiveInt(ContextID ctx, uint32_t ix)
@@ -268,7 +268,7 @@
 /** GICD_IGROUPR{1..31}
  * interrupt group bits for global interrupts
  * 1b per interrupt, 32 bits per word, 31 words */
-uint32_t intGroup[INT_BITS_MAX-1];
+uint32_t intGroup[INT_BITS_MAX - 1] = {};

 uint32_t&
 getIntGroup(ContextID ctx, uint32_t ix)
@@ -282,13 +282,13 @@
 }

 /** read only running priority register, 1 per cpu*/
-uint32_t iccrpr[CPU_MAX];
+uint32_t iccrpr[CPU_MAX] = {};

 /** GICD_IPRIORITYR{8..255}
  * an 8 bit priority (lower is higher priority) for each
  * of the global (not replicated per CPU) interrupts.
  */
-uint8_t intPriority[GLOBAL_INT_LINES];
+uint8_t intPriority[GLOBAL_INT_LINES] = {};

 uint8_t&
 getIntPriority(ContextID ctx, uint32_t ix)
@@ -305,7 +305,7 @@
  * GICD_ICFGR{2...63}
  * 2 bit per interrupt signaling if it's level or edge sensitive
  * and if it is 1:N or N:N */
-uint32_t intConfig[INT_BITS_MAX*2 - 2];
+uint32_t intConfig[INT_BITS_MAX * 2 - 2] = {};

 /**
  * Reads the GICD_ICFGRn register.
@@ -328,7 +328,7 @@
 /** GICD_ITARGETSR{8..255}
  * an 8 bit cpu target id for each global interrupt.
  */
-uint8_t cpuTarget[GLOBAL_INT_LINES];
+uint8_t cpuTarget[GLOBAL_INT_LINES] = {};

 uint8_t
 

[gem5-dev] Change in gem5/gem5[develop]: mem-cache: Don't generate debug output unless you're going to use it.

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



Change subject: mem-cache: Don't generate debug output unless you're going  
to use it.

..

mem-cache: Don't generate debug output unless you're going to use it.

The BaseCache::handleFill function would generate an "old_state" string
unconditionally, just in case it would need to print it out later on in
the function if the Cache debug variable was set. This is very wasteful.
We should only generate that string if we are actually going to use it
later on.

Change-Id: I4a570d1cd2814e5a089eac1233dedd1801d68975
---
M src/mem/cache/base.cc
1 file changed, 16 insertions(+), 1 deletion(-)



diff --git a/src/mem/cache/base.cc b/src/mem/cache/base.cc
index fa9257c..4007a4f 100644
--- a/src/mem/cache/base.cc
+++ b/src/mem/cache/base.cc
@@ -1439,7 +1439,7 @@
 Addr addr = pkt->getAddr();
 bool is_secure = pkt->isSecure();
 const bool has_old_data = blk && blk->isValid();
-const std::string old_state = blk ? blk->print() : "";
+const std::string old_state = (debug::Cache && blk) ?  
blk->print() : "";


 // When handling a fill, we should have no writes to this line.
 assert(addr == pkt->getBlockAddr(blkSize));

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52405
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: I4a570d1cd2814e5a089eac1233dedd1801d68975
Gerrit-Change-Number: 52405
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]: mem-cache: Ensure all fields of the CacheBlk class are initialized.

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



Change subject: mem-cache: Ensure all fields of the CacheBlk class are  
initialized.

..

mem-cache: Ensure all fields of the CacheBlk class are initialized.

The constructor only initialized two fields, data and _tickInserted. The
print() method at least accesses the coherence status bits, which
valgrind determined were being accessed without being initialized.

This change adds a default initializer to all fields to prevent any
value from flapping around uninitialized.

Change-Id: Ie4c839504d49f9a131d8e3c3e8be02ff22f453a6
---
M src/mem/cache/cache_blk.hh
1 file changed, 24 insertions(+), 8 deletions(-)



diff --git a/src/mem/cache/cache_blk.hh b/src/mem/cache/cache_blk.hh
index b3eed9a..775efbe 100644
--- a/src/mem/cache/cache_blk.hh
+++ b/src/mem/cache/cache_blk.hh
@@ -100,13 +100,13 @@
  * data stored here should be kept consistant with the actual data
  * referenced by this block.
  */
-uint8_t *data;
+uint8_t *data = nullptr;

 /**
  * Which curTick() will this block be accessible. Its value is only
  * meaningful if the block is valid.
  */
-Tick whenReady;
+Tick whenReady = 0;

   protected:
 /**
@@ -152,7 +152,7 @@
 std::list lockList;

   public:
-CacheBlk() : TaggedEntry(), data(nullptr), _tickInserted(0)
+CacheBlk()
 {
 invalidate();
 }
@@ -474,22 +474,22 @@

   private:
 /** Task Id associated with this block */
-uint32_t _taskId;
+uint32_t _taskId = 0;

 /** holds the source requestor ID for this block. */
-int _srcRequestorId;
+int _srcRequestorId = 0;

 /** Number of references to this block since it was brought in. */
-unsigned _refCount;
+unsigned _refCount = 0;

 /**
  * Tick on which the block was inserted in the cache. Its value is only
  * meaningful if the block is valid.
  */
-Tick _tickInserted;
+Tick _tickInserted = 0;

 /** Whether this block is an unaccessed hardware prefetch. */
-bool _prefetched;
+bool _prefetched = 0;
 };

 /**

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52404
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: Ie4c839504d49f9a131d8e3c3e8be02ff22f453a6
Gerrit-Change-Number: 52404
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]: dev-arm: Set default initial values in GenericTimer::CoreTimers.

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



Change subject: dev-arm: Set default initial values in  
GenericTimer::CoreTimers.

..

dev-arm: Set default initial values in GenericTimer::CoreTimers.

The cntkctl and cnthctl registers were not initialized by the CoreTimers
constructor which upset valgrind when they were later used by
handleStream.

This change gives them and other simple variables default initial values
so that they will at least have deterministic junk in them if they
aren't sensibly initialized by something else.

Change-Id: Iaedbb2d957aeb428fd563be2e24ccb8d2cf57f26
---
M src/dev/arm/generic_timer.hh
1 file changed, 25 insertions(+), 8 deletions(-)



diff --git a/src/dev/arm/generic_timer.hh b/src/dev/arm/generic_timer.hh
index 9cccef6..cd3dc90 100644
--- a/src/dev/arm/generic_timer.hh
+++ b/src/dev/arm/generic_timer.hh
@@ -311,21 +311,21 @@
 GenericTimer 

 /// System counter frequency as visible from this core
-uint32_t cntfrq;
+uint32_t cntfrq = 0;

 /// Kernel control register
-ArmISA::CNTKCTL cntkctl;
+ArmISA::CNTKCTL cntkctl = 0;

 /// Hypervisor control register
-ArmISA::CNTHCTL cnthctl;
+ArmISA::CNTHCTL cnthctl = 0;

 /// Thread (HW) context associated to this PE implementation
-ThreadContext *threadContext;
+ThreadContext *threadContext = nullptr;

-ArmInterruptPin const *irqPhysS;
-ArmInterruptPin const *irqPhysNS;
-ArmInterruptPin const *irqVirt;
-ArmInterruptPin const *irqHyp;
+ArmInterruptPin const *irqPhysS = nullptr;
+ArmInterruptPin const *irqPhysNS = nullptr;
+ArmInterruptPin const *irqVirt = nullptr;
+ArmInterruptPin const *irqHyp = nullptr;

 ArchTimerKvm physS;
 ArchTimerKvm physNS;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52403
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: Iaedbb2d957aeb428fd563be2e24ccb8d2cf57f26
Gerrit-Change-Number: 52403
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] preview branch for mutli-ISA gem5?

2021-11-04 Thread Gabe Black via gem5-dev
Hey folks, I'm not quite ready for it yet, but I'm getting close to having
everything at least written for a multi-ISA version of gem5. What do you
think about having a preview branch on gerrit? I can create one pretty
easily, I just want to make sure that's appropriate and something other
people might be interested in before I do so. Like I said, I haven't gotten
*quite* to the point where I have something useable, but I think I'm down
to the last few things.

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