[gem5-dev] Re: kokoro failures due to bug in gem5-art?

[Gabe Black via gem5-dev]

One more:

https://source.cloud.google.com/results/invocations/8f6a185e-0258-4f9c-b573-e79d1a86a5a1/targets/gem5%2Fgcp_ubuntu%2Fpresubmit/log

On Thu, Aug 5, 2021 at 3:02 PM Gabe Black  wrote:

> Yeah, I wouldn't be surprised if it was specific to kokoro, or something
> about how its networking is set up. Those failures seem to have stopped
> happening now, or at least are happening much less. I don't remember seeing
> one for a while at least. Hopefully we can avoid making our timeout any
> longer! Thanks for looking into it, Bobby.
>
> Gabe
>
> On Thu, Aug 5, 2021 at 1:16 PM Bobby Bruce  wrote:
>
>> That theory could be true, and I certainly don't have any better ideas,
>> though I've never observed any hang on my local machine when recreating
>> this issue. It could be something specific to Kokoro.
>>
>> I've fixed the gem5art error here:
>> https://gem5-review.googlesource.com/c/public/gem5/+/49044. We can see
>> if this fixes the timeout issue. If the timeout error persists  we can
>> consider increasing the timeout:
>> https://gem5-review.googlesource.com/c/public/gem5/+/48443
>>
>>
>> --
>> Dr. Bobby R. Bruce
>> Room 3050,
>> Kemper Hall, UC Davis
>> Davis,
>> CA, 95616
>>
>> web: https://www.bobbybruce.net
>>
>>
>> On Thu, Jul 22, 2021 at 8:33 PM Gabe Black  wrote:
>>
>>> Another possibility is that while the gem5-art error may not actually
>>> kill the run, it may, for instance, have failed trying to download
>>> something with a generous timeout, and waiting for that timeout pushed the
>>> rest of the run out enough to trip the timeout? Just a thought. I haven't
>>> checked exhaustively, but it feels like the timeout always goes along with
>>> the gem5-art error message.
>>>
>>> Gabe
>>>
>>> On Thu, Jul 22, 2021 at 5:27 PM Gabe Black  wrote:
>>>
 Ok, thanks. I don't know if you saw the CL I put up recently where the
 src/base/cprintftime.cc executable (the one built from that source) was
 broken, which made kokoro fail. The breakage was real and worth fixing, but
 I'm not sure why kokoro was trying to build it in the first place? Maybe
 sometimes kokoro tries building things that we didn't really want it to.

 In my recent scons hacking, I ran into that accidentally when
 build/X86/${BLAHBLAH} expanded into build/X86/ because that variable didn't
 exist, so scons went of and started building EVERYTHING it knew about below
 build/X86/. Hypothetically, that could explain the long build times and the
 building of that random other binary? Maybe we have some sort of race
 condition where a target expands to an empty string?

 Gabe

 On Thu, Jul 22, 2021 at 3:04 PM Bobby Bruce  wrote:

> Ok, so I did look into this today and didn't find anything. On my
> desktop machine the difference in running the pre-submit tests from the
> stable branch and develop branch (including building the binaries) was 
> only
> 10 minutes so we've really not done anything to increase the build/test
> times to a significant extent. My running theory is Kokoro was running
> slower (??? I have no idea what Kokoro is actually doing or running on
> behind the scenes so I don't know whether that makes sense, but I cannot
> think of any other explanation). I don't like the solution, but I've
> submitted a patch to increase the timeout to 7 hours which should stop 
> this
> timeout event from happening:
> https://gem5-review.googlesource.com/c/public/gem5/+/48443
>
> I still haven't looked into the gem5 error yet but I'm pretty
> confident this shouldn't interfere with the presubmit validation.
>
> --
> Dr. Bobby R. Bruce
> Room 3050,
> Kemper Hall, UC Davis
> Davis,
> CA, 95616
>
> web: https://www.bobbybruce.net
>
>
> On Wed, Jul 21, 2021 at 5:37 PM Gabe Black 
> wrote:
>
>> Ok thanks, Bobby. Please let me know if you find anything, especially
>> if it looks like it's a bug in kokoro itself somehow.
>>
>> Gabe
>>
>> On Wed, Jul 21, 2021 at 3:52 PM Bobby Bruce 
>> wrote:
>>
>>> There's definitely something funny going on with the gem5art tests
>>> there but I believe that error is happening without triggering a 
>>> non-zero
>>> exit code. The gem5art test script is set to `set -e`, which means the
>>> script should exit immediately after a failure, yet it doesn't. The 
>>> testing
>>> also continues onto the other tests. I'll look into this.
>>>
>>> In the example you linked, the issue appears to be because it has
>>> reached the 6 hour timeout. We could increase the timeout to fix this, 
>>> but
>>> I'd like to know why our build/test times have increased enough to push 
>>> us
>>> over the 6 hour line.  I'll see if I can figure it out as well.
>>>
>>> --
>>> Dr. Bobby R. Bruce
>>> Room 3050,
>>> Kemper Hall, UC Davis
>>> Davis,

[gem5-dev] Change in gem5/gem5[develop]: scons: Use the os.path prefix when using components of that module.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: scons: Use the os.path prefix when using components of that  
module.

..

scons: Use the os.path prefix when using components of that module.

That makes it obvious where the methods involved are coming from. Also
some of the imported names weren't being used.

Change-Id: I6ec75eef1e5ea9eae51e7df675e477dccb351bd1
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48126
Reviewed-by: Daniel Carvalho 
Reviewed-by: Bobby R. Bruce 
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
---
M src/SConscript
1 file changed, 11 insertions(+), 12 deletions(-)

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



diff --git a/src/SConscript b/src/SConscript
index c8a35b8..0cf361a 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -48,8 +48,6 @@
 import sys
 import zlib

-from os.path import dirname, exists, isdir, isfile, join as joinpath
-
 import SCons

 from gem5_scons import Transform, warning, error
@@ -327,7 +325,7 @@
 '''
 blob_path = os.path.abspath(blob_path)
 blob_out_dir = os.path.join(env['BUILDDIR'], 'blobs')
-path_noext = joinpath(blob_out_dir, symbol)
+path_noext = os.path.join(blob_out_dir, symbol)
 cpp_path = path_noext + '.cc'
 hpp_path = path_noext + '.hh'
 def embedBlob(target, source, env):
@@ -348,7 +346,7 @@
 Source(cpp_path)

 def GdbXml(xml_id, symbol):
-Blob(joinpath(gdb_xml_dir, xml_id), symbol)
+Blob(os.path.join(gdb_xml_dir, xml_id), symbol)

 class Source(SourceFile):
 pass
@@ -380,13 +378,13 @@

 arcpath = path + [ basename ]
 abspath = self.snode.abspath
-if not exists(abspath):
+if not os.path.exists(abspath):
 abspath = self.tnode.abspath

 self.package = package
 self.modname = modname
 self.modpath = modpath
-self.arcname = joinpath(*arcpath)
+self.arcname = os.path.join(*arcpath)
 self.abspath = abspath
 self.compiled = File(self.filename + 'c')
 self.cpp = File(self.filename + '.cc')
@@ -688,15 +686,16 @@
 continue

 if 'SConscript' in files:
-build_dir = joinpath(env['BUILDDIR'], root[len(base_dir) + 1:])
-SConscript(joinpath(root, 'SConscript'), variant_dir=build_dir)
+build_dir = os.path.join(env['BUILDDIR'], root[len(base_dir) + 1:])
+SConscript(os.path.join(root, 'SConscript'), variant_dir=build_dir)

 for extra_dir in extras_dir_list:
-prefix_len = len(dirname(extra_dir)) + 1
+prefix_len = len(os.path.dirname(extra_dir)) + 1

 # Also add the corresponding build directory to pick up generated
 # include files.
-env.Append(CPPPATH=Dir(joinpath(env['BUILDDIR'],  
extra_dir[prefix_len:])))

+env.Append(CPPPATH=Dir(os.path.join(env['BUILDDIR'],
+extra_dir[prefix_len:])))

 for root, dirs, files in os.walk(extra_dir, topdown=True):
 # if build lives in the extras directory, don't walk down it
@@ -704,8 +703,8 @@
 dirs.remove('build')

 if 'SConscript' in files:
-build_dir = joinpath(env['BUILDDIR'], root[prefix_len:])
-SConscript(joinpath(root, 'SConscript'), variant_dir=build_dir)
+build_dir = os.path.join(env['BUILDDIR'], root[prefix_len:])
+SConscript(os.path.join(root, 'SConscript'),  
variant_dir=build_dir)


 for opt in export_vars:
 env.ConfigFile(opt)



2 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/+/48126
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: I6ec75eef1e5ea9eae51e7df675e477dccb351bd1
Gerrit-Change-Number: 48126
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
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]: scons: Delete the comparison operators from SourceFile.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: scons: Delete the comparison operators from SourceFile.
..

scons: Delete the comparison operators from SourceFile.

These are apparently not used and can be deleted.

Change-Id: I201d565d2e0207e0f43e9443ec03c2b695ab
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48125
Reviewed-by: Daniel Carvalho 
Reviewed-by: Bobby R. Bruce 
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
---
M src/SConscript
1 file changed, 0 insertions(+), 7 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, but someone else must approve
  Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/SConscript b/src/SConscript
index df87950..c8a35b8 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -275,13 +275,6 @@
 self.shared_objs[key] = env.SharedObject(self.tnode)
 return self.shared_objs[key]

-def __lt__(self, other): return self.filename < other.filename
-def __le__(self, other): return self.filename <= other.filename
-def __gt__(self, other): return self.filename > other.filename
-def __ge__(self, other): return self.filename >= other.filename
-def __eq__(self, other): return self.filename == other.filename
-def __ne__(self, other): return self.filename != other.filename
-
 def blobToCpp(data, symbol, cpp_code, hpp_code=None, namespace=None):
 '''
 Convert bytes data into C++ .cpp and .hh uint8_t byte array



2 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/+/48125
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: I201d565d2e0207e0f43e9443ec03c2b695ab
Gerrit-Change-Number: 48125
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
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: Eliminate the SourceFile.basename property.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: scons: Eliminate the SourceFile.basename property.
..

scons: Eliminate the SourceFile.basename property.

This value is used in only two places, and can be calculated in place to
avoid complexity.

Change-Id: I1e59b92521250b3f5a3e2cba599236ededf1761d
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48124
Reviewed-by: Daniel Carvalho 
Reviewed-by: Bobby R. Bruce 
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
---
M src/SConscript
1 file changed, 6 insertions(+), 8 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, but someone else must approve
  Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/SConscript b/src/SConscript
index 53df4f5..df87950 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -48,7 +48,7 @@
 import sys
 import zlib

-from os.path import basename, dirname, exists, isdir, isfile, join as  
joinpath

+from os.path import dirname, exists, isdir, isfile, join as joinpath

 import SCons

@@ -275,10 +275,6 @@
 self.shared_objs[key] = env.SharedObject(self.tnode)
 return self.shared_objs[key]

-@property
-def basename(self):
-return basename(self.filename)
-
 def __lt__(self, other): return self.filename < other.filename
 def __le__(self, other): return self.filename <= other.filename
 def __gt__(self, other): return self.filename > other.filename
@@ -375,7 +371,8 @@
 '''specify the python package, the source file, and any tags'''
 super(PySource, self).__init__(source, tags, add_tags)

-modname, ext = os.path.splitext(self.basename)
+basename = os.path.basename(self.filename)
+modname, ext = os.path.splitext(basename)
 assert ext == '.py'

 if package:
@@ -388,7 +385,7 @@
 modpath += [ modname ]
 modpath = '.'.join(modpath)

-arcpath = path + [ self.basename ]
+arcpath = path + [ basename ]
 abspath = self.snode.abspath
 if not exists(abspath):
 abspath = self.tnode.abspath
@@ -461,7 +458,8 @@
 error('Got protobuf to build, but lacks support!')

 # Get the file name and the extension
-modname, ext = os.path.splitext(self.basename)
+basename = os.path.basename(self.filename)
+modname, ext = os.path.splitext(basename)
 assert ext == '.proto'

 self.cc_file, self.hh_file = env.ProtoBufCC(source=source)



2 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/+/48124
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: I1e59b92521250b3f5a3e2cba599236ededf1761d
Gerrit-Change-Number: 48124
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
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: Replace the SourceFile.filename property with attribute.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: scons: Replace the SourceFile.filename property with  
attribute.

..

scons: Replace the SourceFile.filename property with attribute.

The SourceFile.filename property dynamically calculated the str()
conversion of self.tnode. Since self.tnode shouldn't be changed, it
doesn't seem useful to calculate that value over and over, especially
since it adds some extra indirection and magic to something that's
really pretty simple.

Change-Id: Ia0e1e8f4b0c019a026a08b5c2730d93c66de8190
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48123
Reviewed-by: Daniel Carvalho 
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
---
M src/SConscript
1 file changed, 1 insertion(+), 4 deletions(-)

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



diff --git a/src/SConscript b/src/SConscript
index 1565cd3..53df4f5 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -250,6 +250,7 @@
 tnode = File(source)

 self.tnode = tnode
+self.filename = str(self.tnode)
 self.snode = tnode.srcnode()

 for base in type(self).__mro__:
@@ -275,10 +276,6 @@
 return self.shared_objs[key]

 @property
-def filename(self):
-return str(self.tnode)
-
-@property
 def basename(self):
 return basename(self.filename)




2 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/+/48123
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: Ia0e1e8f4b0c019a026a08b5c2730d93c66de8190
Gerrit-Change-Number: 48123
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
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: Replace the extname property with os.path.splitext().

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: scons: Replace the extname property with os.path.splitext().
..

scons: Replace the extname property with os.path.splitext().

This is almost exactly the same, except it leaves the "." on the
extension, and returns an empty string instead of None if there is no
extension.

Change-Id: Idb540771007f9f7ca8aafdb09512eb1219010237
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48122
Maintainer: Gabe Black 
Tested-by: kokoro 
Reviewed-by: Daniel Carvalho 
---
M src/SConscript
1 file changed, 5 insertions(+), 13 deletions(-)

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



diff --git a/src/SConscript b/src/SConscript
index a4b35cd..1565cd3 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -43,6 +43,7 @@
 import functools
 import imp
 import os
+import os.path
 import re
 import sys
 import zlib
@@ -281,15 +282,6 @@
 def basename(self):
 return basename(self.filename)

-@property
-def extname(self):
-index = self.basename.rfind('.')
-if index <= 0:
-# dot files aren't extensions
-return self.basename, None
-
-return self.basename[:index], self.basename[index+1:]
-
 def __lt__(self, other): return self.filename < other.filename
 def __le__(self, other): return self.filename <= other.filename
 def __gt__(self, other): return self.filename > other.filename
@@ -386,8 +378,8 @@
 '''specify the python package, the source file, and any tags'''
 super(PySource, self).__init__(source, tags, add_tags)

-modname,ext = self.extname
-assert ext == 'py'
+modname, ext = os.path.splitext(self.basename)
+assert ext == '.py'

 if package:
 path = package.split('.')
@@ -472,8 +464,8 @@
 error('Got protobuf to build, but lacks support!')

 # Get the file name and the extension
-modname,ext = self.extname
-assert ext == 'proto'
+modname, ext = os.path.splitext(self.basename)
+assert ext == '.proto'

 self.cc_file, self.hh_file = env.ProtoBufCC(source=source)




2 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/+/48122
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: Idb540771007f9f7ca8aafdb09512eb1219010237
Gerrit-Change-Number: 48122
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Gabe Black 
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]: scons: Make all Executables strip-able, and de-special case .fast.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: scons: Make all Executables strip-able, and de-special  
case .fast.

..

scons: Make all Executables strip-able, and de-special case .fast.

The build for .fast was set up to produce a stripped executable, and the
unstripped executable was instead named .fast.unstripped. I think the
assumption that a stripped executable is faster than an unstripped
executable is flawed, since the parts of the binary that are removed,
debug symbols, are not loaded into memory anyway, so while the program
is executing it shouldn't be any different or take up any additional
memory. This also made .fast a special case compared to the other build
types, like .opt, .debug, etc.

Instead, this change makes .fast unstripped like all the other binaries,
and also makes it possible to request a stripped version of *any* binary
the build can produce with a .stripped suffix.

Change-Id: I2de82e0951d9f41c30594f32fba50acdd14ed69c
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48120
Reviewed-by: Bobby R. Bruce 
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
---
M src/SConscript
1 file changed, 11 insertions(+), 14 deletions(-)

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



diff --git a/src/SConscript b/src/SConscript
index f79a657..e4f0c42 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -576,18 +576,16 @@
 env['BIN_RPATH_PREFIX'] = os.path.relpath(
 env['BUILDDIR'], self.path(env).dir.abspath)

-if env['STRIP_EXES']:
-stripped = self.path(env)
-unstripped = env.File(str(stripped) + '.unstripped')
-if sys.platform == 'sunos5':
-cmd = 'cp $SOURCE $TARGET; strip $TARGET'
-else:
-cmd = 'strip $SOURCE -o $TARGET'
-env.Program(unstripped, objs)
-return env.Command(stripped, unstripped,
-   MakeAction(cmd, Transform("STRIP")))
+executable = env.Program(self.path(env), objs)[0]
+
+if sys.platform == 'sunos5':
+cmd = 'cp $SOURCE $TARGET; strip $TARGET'
 else:
-return env.Program(self.path(env), objs)
+cmd = 'strip $SOURCE -o $TARGET'
+stripped = env.Command(str(executable) + '.stripped',
+executable, MakeAction(cmd, Transform("STRIP")))[0]
+
+return [executable, stripped]

 class GTest(Executable):
 '''Create a unit test based on the google test framework.'''
@@ -1344,7 +1342,7 @@
 # environment 'env' with modified object suffix and optional stripped
 # binary.  Additional keyword arguments are appended to corresponding
 # build environment vars.
-def makeEnv(env, label, objsfx, strip=False, **kwargs):
+def makeEnv(env, label, objsfx, **kwargs):
 # SCons doesn't know to append a library suffix when there is a '.' in  
the

 # name.  Use '_' instead.
 libname = 'gem5_' + label
@@ -1390,7 +1388,6 @@

 # Record some settings for building Executables.
 new_env['EXE_SUFFIX'] = label
-new_env['STRIP_EXES'] = strip

 for cls in ExecutableMeta.all:
 cls.declare_all(new_env)
@@ -1469,7 +1466,7 @@

 # "Fast" binary
 if 'fast' in needed_envs:
-makeEnv(env, 'fast', '.fo', strip = True,
+makeEnv(env, 'fast', '.fo',
 CCFLAGS = Split(ccflags['fast']),
 CPPDEFINES = ['NDEBUG', 'TRACING_ON=0'],
 LINKFLAGS = Split(ldflags['fast']))

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48120
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: I2de82e0951d9f41c30594f32fba50acdd14ed69c
Gerrit-Change-Number: 48120
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Gabe Black 
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: Get rid of the unused "dirname" property of SourceFile.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: scons: Get rid of the unused "dirname" property of  
SourceFile.

..

scons: Get rid of the unused "dirname" property of SourceFile.

Change-Id: I7c52f866542057b9f11ba96434f9c6f93ff0ea46
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48121
Reviewed-by: Bobby R. Bruce 
Maintainer: Bobby R. Bruce 
Tested-by: kokoro 
---
M src/SConscript
1 file changed, 0 insertions(+), 4 deletions(-)

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



diff --git a/src/SConscript b/src/SConscript
index e4f0c42..a4b35cd 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -278,10 +278,6 @@
 return str(self.tnode)

 @property
-def dirname(self):
-return dirname(self.filename)
-
-@property
 def basename(self):
 return basename(self.filename)


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48121
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: I7c52f866542057b9f11ba96434f9c6f93ff0ea46
Gerrit-Change-Number: 48121
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Gabe Black 
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] Running clang-format

[Jason Lowe-Power via gem5-dev]

Hi everyone,

For fun(?), I have come up with a clang-format file that's pretty darn
close to our current style.

However, the problem is that most of our files don't actually meet our
style guide! :'( So, running this clang-format generates an enormous
changeset. Skimming through the (100,000+) changes clang-format makes all
of them look "correct" to me. But I didn't check them all, just a few
spot-checks.

I've put together a WIP commit as an example of what it looks like to run
clang-format on gem5. There are a few caveats, however.

1. BitUnion really confuses clang-format for two reasons: (1) We indent the
code inside the BitUntion macro. (2) there are no semicolons after the
macro calls for BitUnion and EndBitUntion.
2. There are a few lines that go over 79 characters, but they're easy to
manually fix (it's only 4 files).
3. When I ran style.py on all files in src/ there were a huge number that
had invalid sorting of includes. In at least one case, this causes compiler
failures when I let style.py fix it.
4. I'm skipping sorting includes with clang-format right now. There is a
way to specify the blocks that we define, but I don't want to dive into
this right now. I'll create a Jira issue so we can come back to it.

Finally, I'm using the following shell script to test things. This runs sed
to tell clang-format to ignore the BitUnion blocks, runs clang-format, and
then undoes the sed script.

If this seems like something we want to continue with, then I'll commit
these changes and we can add clang-format to the commit hooks.

Clang-format file:
https://gem5-review.googlesource.com/c/public/gem5/+/49063
The giant changeset:
https://gem5-review.googlesource.com/c/public/gem5/+/49064/

The script I used (don't judge my sed ability :D):
```
# Make clang-format ignore bitunions
grep -r -l BitUnion src | xargs sed -i -e '/^ *BitUnion.*(.*)/i \
// clang-format off' -e '/^ *EndBitUnion(.*)/a; \
// clang-format on'

# undo the change to the bitunion file
git checkout src/base/bitunion.hh

# Run clang format
find src -name "*.cc" -or -name "*.hh" -exec clang-format -style=file -i {}
\;

# Remove clang-format off in BitUnion files
grep -r -l BitUnion src | xargs sed -i -e '/^ *\/\/ clang-format.*$/d' -e
'/^; *$/d'
```

Cheers,
Jason
___
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: python versions used by SCons and gem5

[Jason Lowe-Power via gem5-dev]

For MyPy that won't help.. the problem is that it looks for
_m5/objects/.py *file* which doesn't exist on disk... we have to
somehow convince it to look in the marshalled data. I was playing around
with this, but I haven't figured it out yet.

BTW, your suggested changes get a +1 from me.

Cheers,
Jason

On Thu, Aug 5, 2021 at 4:00 PM Gabe Black via gem5-dev 
wrote:

>
>
> On Thu, Aug 5, 2021 at 3:08 PM Bobby Bruce  wrote:
>
>> Hey Gabe,
>>
>> Just so we're on the same page, I'd like to take a moment to outline how
>> I understand this problem/solution. Please let me know if I've understood
>> correctly:
>>
>> We essentially have three components of the project which interpret gem5
>> Python code: The gem5 binary, the Marshal binary, and the Scons build
>> system. The gem5 and Marshal binaries both link to the Python library, so
>> we have no problems there; they should interpret things identically. The
>> problematic entity is Scons which will use the host python executable when
>> processing Simobjects et al. at compile time. Your solution is to offload
>> the relevant Scons gem5 Python interpretations/auto-generations to the
>> Marshal binary where they'll use the Python library. Is this correct?
>>
>
> Correct.
>
>
>>
>> If so, I support this move as it makes things considerably easier to work
>> with. While there is the small `--without-python` drawback, I don't
>> personally see this being a big problem for the vast majority of users.
>>
>
> Ok, great. I'll move forward with that then.
>
>
>>
>>
>> I do have a few of questions (I'm not sure if you an answer them, but may
>> be something to think about):
>>
>> 1) Will this make it easier to embed configuration scripts (such as those
>> found in the `configs` directory) into the gem5 binary?
>>
>
> That would be orthogonal to this. You could more or less do that today,
> although I think in most cases you wouldn't want to since then you'd have
> to recompile gem5 to change them.
>
>
>> 2) Will this change the way we add new SimObjects or will it be
>> more-or-less identical as before?
>>
>
> There should be no change. Unrelated to this, the more I dig around in how
> SimObjects are set up the more I think we may need to change something
> there, but I haven't thought about that enough to know for sure if that's
> necessary, or what a better system might look like.
>
>
>> 3) One of our goals moving forward is to enforce more Python type
>> annotations so we can use tools such as MyPy. We've had some problems
>> running this with this due to embedding (i.e., we can't just point MyPy at
>> a SimObject Python file). Do you have any idea if this could be made easier?
>>
>
>
> I'm not familiar with mypy, but looking at their website, something like
> this might work?
>
> https://mypy.readthedocs.io/en/latest/extending_mypy.html
>
> It looks like it would be fairly easy to add that to gem5's python "main"
> method, although of course I haven't tried it.
>
> 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] Re: python versions used by SCons and gem5

[Gabe Black via gem5-dev]

On Thu, Aug 5, 2021 at 3:08 PM Bobby Bruce  wrote:

> Hey Gabe,
>
> Just so we're on the same page, I'd like to take a moment to outline how I
> understand this problem/solution. Please let me know if I've understood
> correctly:
>
> We essentially have three components of the project which interpret gem5
> Python code: The gem5 binary, the Marshal binary, and the Scons build
> system. The gem5 and Marshal binaries both link to the Python library, so
> we have no problems there; they should interpret things identically. The
> problematic entity is Scons which will use the host python executable when
> processing Simobjects et al. at compile time. Your solution is to offload
> the relevant Scons gem5 Python interpretations/auto-generations to the
> Marshal binary where they'll use the Python library. Is this correct?
>

Correct.


>
> If so, I support this move as it makes things considerably easier to work
> with. While there is the small `--without-python` drawback, I don't
> personally see this being a big problem for the vast majority of users.
>

Ok, great. I'll move forward with that then.


>
>
> I do have a few of questions (I'm not sure if you an answer them, but may
> be something to think about):
>
> 1) Will this make it easier to embed configuration scripts (such as those
> found in the `configs` directory) into the gem5 binary?
>

That would be orthogonal to this. You could more or less do that today,
although I think in most cases you wouldn't want to since then you'd have
to recompile gem5 to change them.


> 2) Will this change the way we add new SimObjects or will it be
> more-or-less identical as before?
>

There should be no change. Unrelated to this, the more I dig around in how
SimObjects are set up the more I think we may need to change something
there, but I haven't thought about that enough to know for sure if that's
necessary, or what a better system might look like.


> 3) One of our goals moving forward is to enforce more Python type
> annotations so we can use tools such as MyPy. We've had some problems
> running this with this due to embedding (i.e., we can't just point MyPy at
> a SimObject Python file). Do you have any idea if this could be made easier?
>


I'm not familiar with mypy, but looking at their website, something like
this might work?

https://mypy.readthedocs.io/en/latest/extending_mypy.html

It looks like it would be fairly easy to add that to gem5's python "main"
method, although of course I haven't tried it.

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: python versions used by SCons and gem5

[Bobby Bruce via gem5-dev]

Hey Gabe,

Just so we're on the same page, I'd like to take a moment to outline how I
understand this problem/solution. Please let me know if I've understood
correctly:

We essentially have three components of the project which interpret gem5
Python code: The gem5 binary, the Marshal binary, and the Scons build
system. The gem5 and Marshal binaries both link to the Python library, so
we have no problems there; they should interpret things identically. The
problematic entity is Scons which will use the host python executable when
processing Simobjects et al. at compile time. Your solution is to offload
the relevant Scons gem5 Python interpretations/auto-generations to the
Marshal binary where they'll use the Python library. Is this correct?

If so, I support this move as it makes things considerably easier to work
with. While there is the small `--without-python` drawback, I don't
personally see this being a big problem for the vast majority of users.

I do have a few of questions (I'm not sure if you an answer them, but may
be something to think about):

1) Will this make it easier to embed configuration scripts (such as those
found in the `configs` directory) into the gem5 binary?
2) Will this change the way we add new SimObjects or will it be
more-or-less identical as before?
3) One of our goals moving forward is to enforce more Python type
annotations so we can use tools such as MyPy. We've had some problems
running this with this due to embedding (i.e., we can't just point MyPy at
a SimObject Python file). Do you have any idea if this could be made easier?

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

web: https://www.bobbybruce.net


On Fri, Jul 30, 2021 at 11:05 PM Gabe Black via gem5-dev 
wrote:

> Hi folks, particularly Bobby who made a change related to this fairly
> recently. Please offer some feedback, or I'll be forced to assume your
> silence is tacit agreement.
>
> Gabe
>
> On Wed, Jul 28, 2021 at 8:30 PM Gabe Black  wrote:
>
>> Bump...
>>
>> On Sat, Jul 24, 2021 at 1:16 AM Gabe Black  wrote:
>>
>>> Hi folks.
>>>
>>> I think often when we think of how python is used by gem5, many people
>>> (myself included) tend to think of it as a monolithic entity, or in other
>>> words that there is just *the* python that gets used by everything. The
>>> real situation is a little more complicated than that.
>>>
>>> On the one hand, we have "the" version of the python interpreter itself,
>>> which is used to directly run python scripts. This is again an
>>> approximation because we could, for instance, have one version of python
>>> running SCons itself, and another version running scripts gem5 provides,
>>> but for simplicity let's assume that's not the case, or at least that it
>>> doesn't matter.
>>>
>>> Then, we have the python libraries which get built into gem5 and provide
>>> it's embedded interpreter. This may not exist at all on the target system,
>>> or it could exist but be a different version than the one which you'd get
>>> by running the command line "python foo.py".
>>>
>>>
>>> **Marshal binary**
>>>
>>> The first place this has caused complication is where gem5's python code
>>> is packaged up by the build system so that it can be embedded into gem5 in
>>> either library form or executable form. Through digging around in the
>>> repository, I've found that a long, long time ago, gem5 was only a binary,
>>> and the python code was embedded into it by tacking a zip archive onto the
>>> end where gem5 could find it. Apparently this doesn't work for libraries,
>>> and so the python code needed to be bundled up and more formally injected
>>> into the binary as data. Today done by turning the python code into a
>>> binary representation, compressing it into a zip file, and then building
>>> the resulting file into a byte array declared in c++.
>>>
>>> There are, to the first order, two main ways to serialize python "stuff"
>>> into a string/byte array, pickling or marshalling. Both have their
>>> advantages and disadvantages, partially covered here:
>>>
>>> https://docs.python.org/3/library/pickle.html#module-pickle
>>>
>>> The main deal breaker disadvantage of the pickle module is that it does
>>> *not* handle code, and so all the code inside our python modules would be
>>> lost, leaving only the data. The main disadvantage of marshal is that there
>>> is *no* guarantee that it will be compatible between different versions of
>>> python, since it's intended as a mostly internal representation, for, for
>>> instance ".pyc" files.
>>>
>>> To solve this problem, Andreas added a "marshal" binary which is written
>>> in c++, and includes the python interpreter like gem5 itself would. It runs
>>> a small embedded script to use the appropriate version of the marshal
>>> module to serialize the code we want. Since the code runs under the same
>>> version of the interpreter as gem5 itself would use, when gem5 tries to use
>>> the data it will be 

[gem5-dev] Re: kokoro failures due to bug in gem5-art?

[Gabe Black via gem5-dev]

Yeah, I wouldn't be surprised if it was specific to kokoro, or something
about how its networking is set up. Those failures seem to have stopped
happening now, or at least are happening much less. I don't remember seeing
one for a while at least. Hopefully we can avoid making our timeout any
longer! Thanks for looking into it, Bobby.

Gabe

On Thu, Aug 5, 2021 at 1:16 PM Bobby Bruce  wrote:

> That theory could be true, and I certainly don't have any better ideas,
> though I've never observed any hang on my local machine when recreating
> this issue. It could be something specific to Kokoro.
>
> I've fixed the gem5art error here:
> https://gem5-review.googlesource.com/c/public/gem5/+/49044. We can see if
> this fixes the timeout issue. If the timeout error persists  we can
> consider increasing the timeout:
> https://gem5-review.googlesource.com/c/public/gem5/+/48443
>
>
> --
> Dr. Bobby R. Bruce
> Room 3050,
> Kemper Hall, UC Davis
> Davis,
> CA, 95616
>
> web: https://www.bobbybruce.net
>
>
> On Thu, Jul 22, 2021 at 8:33 PM Gabe Black  wrote:
>
>> Another possibility is that while the gem5-art error may not actually
>> kill the run, it may, for instance, have failed trying to download
>> something with a generous timeout, and waiting for that timeout pushed the
>> rest of the run out enough to trip the timeout? Just a thought. I haven't
>> checked exhaustively, but it feels like the timeout always goes along with
>> the gem5-art error message.
>>
>> Gabe
>>
>> On Thu, Jul 22, 2021 at 5:27 PM Gabe Black  wrote:
>>
>>> Ok, thanks. I don't know if you saw the CL I put up recently where the
>>> src/base/cprintftime.cc executable (the one built from that source) was
>>> broken, which made kokoro fail. The breakage was real and worth fixing, but
>>> I'm not sure why kokoro was trying to build it in the first place? Maybe
>>> sometimes kokoro tries building things that we didn't really want it to.
>>>
>>> In my recent scons hacking, I ran into that accidentally when
>>> build/X86/${BLAHBLAH} expanded into build/X86/ because that variable didn't
>>> exist, so scons went of and started building EVERYTHING it knew about below
>>> build/X86/. Hypothetically, that could explain the long build times and the
>>> building of that random other binary? Maybe we have some sort of race
>>> condition where a target expands to an empty string?
>>>
>>> Gabe
>>>
>>> On Thu, Jul 22, 2021 at 3:04 PM Bobby Bruce  wrote:
>>>
 Ok, so I did look into this today and didn't find anything. On my
 desktop machine the difference in running the pre-submit tests from the
 stable branch and develop branch (including building the binaries) was only
 10 minutes so we've really not done anything to increase the build/test
 times to a significant extent. My running theory is Kokoro was running
 slower (??? I have no idea what Kokoro is actually doing or running on
 behind the scenes so I don't know whether that makes sense, but I cannot
 think of any other explanation). I don't like the solution, but I've
 submitted a patch to increase the timeout to 7 hours which should stop this
 timeout event from happening:
 https://gem5-review.googlesource.com/c/public/gem5/+/48443

 I still haven't looked into the gem5 error yet but I'm pretty confident
 this shouldn't interfere with the presubmit validation.

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

 web: https://www.bobbybruce.net


 On Wed, Jul 21, 2021 at 5:37 PM Gabe Black 
 wrote:

> Ok thanks, Bobby. Please let me know if you find anything, especially
> if it looks like it's a bug in kokoro itself somehow.
>
> Gabe
>
> On Wed, Jul 21, 2021 at 3:52 PM Bobby Bruce 
> wrote:
>
>> There's definitely something funny going on with the gem5art tests
>> there but I believe that error is happening without triggering a non-zero
>> exit code. The gem5art test script is set to `set -e`, which means the
>> script should exit immediately after a failure, yet it doesn't. The 
>> testing
>> also continues onto the other tests. I'll look into this.
>>
>> In the example you linked, the issue appears to be because it has
>> reached the 6 hour timeout. We could increase the timeout to fix this, 
>> but
>> I'd like to know why our build/test times have increased enough to push 
>> us
>> over the 6 hour line.  I'll see if I can figure it out as well.
>>
>> --
>> Dr. Bobby R. Bruce
>> Room 3050,
>> Kemper Hall, UC Davis
>> Davis,
>> CA, 95616
>>
>> web: https://www.bobbybruce.net
>>
>>
>> On Wed, Jul 21, 2021 at 2:51 PM Gabe Black via gem5-dev <
>> gem5-dev@gem5.org> wrote:
>>
>>> I've seen many kokoro failures lately, including this one which
>>> seems to be from a problem in gem5-art? Any idea what's going on?
>>>
>>>

[gem5-dev] Re: kokoro failures due to bug in gem5-art?

[Bobby Bruce via gem5-dev]

That theory could be true, and I certainly don't have any better ideas,
though I've never observed any hang on my local machine when recreating
this issue. It could be something specific to Kokoro.

I've fixed the gem5art error here:
https://gem5-review.googlesource.com/c/public/gem5/+/49044. We can see if
this fixes the timeout issue. If the timeout error persists  we can
consider increasing the timeout:
https://gem5-review.googlesource.com/c/public/gem5/+/48443


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

web: https://www.bobbybruce.net


On Thu, Jul 22, 2021 at 8:33 PM Gabe Black  wrote:

> Another possibility is that while the gem5-art error may not actually kill
> the run, it may, for instance, have failed trying to download something
> with a generous timeout, and waiting for that timeout pushed the rest of
> the run out enough to trip the timeout? Just a thought. I haven't checked
> exhaustively, but it feels like the timeout always goes along with the
> gem5-art error message.
>
> Gabe
>
> On Thu, Jul 22, 2021 at 5:27 PM Gabe Black  wrote:
>
>> Ok, thanks. I don't know if you saw the CL I put up recently where the
>> src/base/cprintftime.cc executable (the one built from that source) was
>> broken, which made kokoro fail. The breakage was real and worth fixing, but
>> I'm not sure why kokoro was trying to build it in the first place? Maybe
>> sometimes kokoro tries building things that we didn't really want it to.
>>
>> In my recent scons hacking, I ran into that accidentally when
>> build/X86/${BLAHBLAH} expanded into build/X86/ because that variable didn't
>> exist, so scons went of and started building EVERYTHING it knew about below
>> build/X86/. Hypothetically, that could explain the long build times and the
>> building of that random other binary? Maybe we have some sort of race
>> condition where a target expands to an empty string?
>>
>> Gabe
>>
>> On Thu, Jul 22, 2021 at 3:04 PM Bobby Bruce  wrote:
>>
>>> Ok, so I did look into this today and didn't find anything. On my
>>> desktop machine the difference in running the pre-submit tests from the
>>> stable branch and develop branch (including building the binaries) was only
>>> 10 minutes so we've really not done anything to increase the build/test
>>> times to a significant extent. My running theory is Kokoro was running
>>> slower (??? I have no idea what Kokoro is actually doing or running on
>>> behind the scenes so I don't know whether that makes sense, but I cannot
>>> think of any other explanation). I don't like the solution, but I've
>>> submitted a patch to increase the timeout to 7 hours which should stop this
>>> timeout event from happening:
>>> https://gem5-review.googlesource.com/c/public/gem5/+/48443
>>>
>>> I still haven't looked into the gem5 error yet but I'm pretty confident
>>> this shouldn't interfere with the presubmit validation.
>>>
>>> --
>>> Dr. Bobby R. Bruce
>>> Room 3050,
>>> Kemper Hall, UC Davis
>>> Davis,
>>> CA, 95616
>>>
>>> web: https://www.bobbybruce.net
>>>
>>>
>>> On Wed, Jul 21, 2021 at 5:37 PM Gabe Black  wrote:
>>>
 Ok thanks, Bobby. Please let me know if you find anything, especially
 if it looks like it's a bug in kokoro itself somehow.

 Gabe

 On Wed, Jul 21, 2021 at 3:52 PM Bobby Bruce  wrote:

> There's definitely something funny going on with the gem5art tests
> there but I believe that error is happening without triggering a non-zero
> exit code. The gem5art test script is set to `set -e`, which means the
> script should exit immediately after a failure, yet it doesn't. The 
> testing
> also continues onto the other tests. I'll look into this.
>
> In the example you linked, the issue appears to be because it has
> reached the 6 hour timeout. We could increase the timeout to fix this, but
> I'd like to know why our build/test times have increased enough to push us
> over the 6 hour line.  I'll see if I can figure it out as well.
>
> --
> Dr. Bobby R. Bruce
> Room 3050,
> Kemper Hall, UC Davis
> Davis,
> CA, 95616
>
> web: https://www.bobbybruce.net
>
>
> On Wed, Jul 21, 2021 at 2:51 PM Gabe Black via gem5-dev <
> gem5-dev@gem5.org> wrote:
>
>> I've seen many kokoro failures lately, including this one which seems
>> to be from a problem in gem5-art? Any idea what's going on?
>>
>>
>> https://source.cloud.google.com/results/invocations/caae5aad-91a6-4c6e-9fbe-20962f9c5519/targets/gem5%2Fgcp_ubuntu%2Fpresubmit/log
>> ___
>> 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

[gem5-dev] Change in gem5/gem5[develop]: tests: Remove setuptools upgrade in gem5art-tests.sh

[Bobby R. Bruce (Gerrit) via gem5-dev]

Bobby R. Bruce has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/49043 )



Change subject: tests: Remove setuptools upgrade in gem5art-tests.sh
..

tests: Remove setuptools upgrade in gem5art-tests.sh

This upgrade is no longer needed as we test using the Ubuntu 20.04 image
which comes with the correct, up-to-date version of setup tools.

Change-Id: I194f26381f59f5ca3edea46696e6d5b2b0418719
---
M tests/jenkins/gem5art-tests.sh
1 file changed, 0 insertions(+), 5 deletions(-)



diff --git a/tests/jenkins/gem5art-tests.sh b/tests/jenkins/gem5art-tests.sh
index 2461de6..c655fbb 100755
--- a/tests/jenkins/gem5art-tests.sh
+++ b/tests/jenkins/gem5art-tests.sh
@@ -43,11 +43,6 @@
 python3 -m venv .pyenv
 source .pyenv/bin/activate

-# The 18.04_all-depenencies image has an outdated version of setuptools. We
-# update it here. Attempts to fix this via the Dockerfile have been
-# unsuccessful.
-pip install -U setuptools
-
 # Install the packages
 pip install -e util/gem5art/artifact
 pip install -e util/gem5art/run

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49043
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: I194f26381f59f5ca3edea46696e6d5b2b0418719
Gerrit-Change-Number: 49043
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] Change in gem5/gem5[develop]: tests: Update pip prior to installing gem5art packages

[Bobby R. Bruce (Gerrit) via gem5-dev]

Bobby R. Bruce has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/49044 )



Change subject: tests: Update pip prior to installing gem5art packages
..

tests: Update pip prior to installing gem5art packages

This fixes the `invalid command 'bdist_wheel'` error which may be
causing wider problems, as reported in this email thread:
https://www.mail-archive.com/gem5-dev@gem5.org/msg39790.html

The `bdist_wheel` package, a dependency of the `celery` package, was
not being installed correctly. The root cause of this
problem is not known, but solutions are presented here:
https://stackoverflow.com/questions/34819221/why-is-python-setup-py-saying-invalid-command-bdist-wheel-on-travis-ci

Upgrading pip prior to installing the gem5art packages seemed like the
simplest solution.

Change-Id: I1bd4e85871c37d0522e16a90ff6b8acc3fdd9f94
---
M tests/jenkins/gem5art-tests.sh
1 file changed, 8 insertions(+), 0 deletions(-)



diff --git a/tests/jenkins/gem5art-tests.sh b/tests/jenkins/gem5art-tests.sh
index c655fbb..b9dee55 100755
--- a/tests/jenkins/gem5art-tests.sh
+++ b/tests/jenkins/gem5art-tests.sh
@@ -43,6 +43,14 @@
 python3 -m venv .pyenv
 source .pyenv/bin/activate

+# The 20.04_all-dependencies image has a slightly outdated version of pip  
which

+# causes problems when trying to install the celery package. The error
+# is `invalid command 'bdist_wheel'`, which can be resolved by upgrading  
pip

+# prior to installing the modules. More information on this error is found
+# here:
+#  
https://stackoverflow.com/questions/34819221/why-is-python-setup-py-saying-invalid-command-bdist-wheel-on-travis-ci

+ pip install --upgrade pip
+
 # Install the packages
 pip install -e util/gem5art/artifact
 pip install -e util/gem5art/run

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49044
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: I1bd4e85871c37d0522e16a90ff6b8acc3fdd9f94
Gerrit-Change-Number: 49044
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] Change in gem5/gem5[develop]: arch,cpu: Rename RegClass to RegClassType.

[Gabe Black (Gerrit) via gem5-dev]

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


Change subject: arch,cpu: Rename RegClass to RegClassType.
..

arch,cpu: Rename RegClass to RegClassType.

This type is really an index which selects a RegClass, not a RegClass
itself.

A follow on change will rename RegClassInfo to RegClass.

Change-Id: I2c1b1d4105bd11b58680053b484d4c1aa1055a9f
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/45229
Maintainer: Gabe Black 
Maintainer: Giacomo Travaglini 
Reviewed-by: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/arch/arm/tracers/tarmac_record.hh
M src/cpu/o3/regfile.cc
M src/cpu/o3/regfile.hh
M src/cpu/reg_class.hh
4 files changed, 12 insertions(+), 11 deletions(-)

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



diff --git a/src/arch/arm/tracers/tarmac_record.hh  
b/src/arch/arm/tracers/tarmac_record.hh

index a1d42b2..fb45e9e 100644
--- a/src/arch/arm/tracers/tarmac_record.hh
+++ b/src/arch/arm/tracers/tarmac_record.hh
@@ -162,7 +162,7 @@
 /** True if register entry is valid */
 bool regValid;
 /** Register class */
-RegClass regClass;
+RegClassType regClass;
 /** Register arch number */
 RegIndex regRel;
 /** Register name to be printed */
diff --git a/src/cpu/o3/regfile.cc b/src/cpu/o3/regfile.cc
index df9dd78..f2f0111 100644
--- a/src/cpu/o3/regfile.cc
+++ b/src/cpu/o3/regfile.cc
@@ -196,7 +196,7 @@
 }

 PhysRegFile::IdRange
-PhysRegFile::getRegIds(RegClass cls)
+PhysRegFile::getRegIds(RegClassType cls)
 {
 switch (cls)
 {
diff --git a/src/cpu/o3/regfile.hh b/src/cpu/o3/regfile.hh
index c7cf807..8101d53 100644
--- a/src/cpu/o3/regfile.hh
+++ b/src/cpu/o3/regfile.hh
@@ -357,7 +357,7 @@
  * Auxiliary function to transition from Full vector mode to Elem mode
  * and to initialise the rename map.
  */
-IdRange getRegIds(RegClass cls);
+IdRange getRegIds(RegClassType cls);

 /**
  * Get the true physical register id.
diff --git a/src/cpu/reg_class.hh b/src/cpu/reg_class.hh
index 0b57487..f7a4d1c 100644
--- a/src/cpu/reg_class.hh
+++ b/src/cpu/reg_class.hh
@@ -53,7 +53,7 @@
 {

 /** Enumerate the classes of registers. */
-enum RegClass
+enum RegClassType
 {
 IntRegClass,///< Integer register
 FloatRegClass,  ///< Floating-point register
@@ -114,7 +114,7 @@
 {
   protected:
 static const char* regClassStrings[];
-RegClass regClass;
+RegClassType regClass;
 RegIndex regIdx;
 ElemIndex elemIdx;
 static constexpr size_t Scale = TheISA::NumVecElemPerVecReg;
@@ -125,10 +125,11 @@
   public:
 RegId() : RegId(IntRegClass, 0) {}

-RegId(RegClass reg_class, RegIndex reg_idx)
+RegId(RegClassType reg_class, RegIndex reg_idx)
 : RegId(reg_class, reg_idx, IllegalElemIndex) {}

-explicit RegId(RegClass reg_class, RegIndex reg_idx, ElemIndex  
elem_idx)

+explicit RegId(RegClassType reg_class, RegIndex reg_idx,
+ElemIndex elem_idx)
 : regClass(reg_class), regIdx(reg_idx), elemIdx(elem_idx),
   numPinnedWrites(0)
 {
@@ -172,7 +173,7 @@
 }

 /** @return true if it is of the specified class. */
-bool is(RegClass reg_class) const { return regClass == reg_class; }
+bool is(RegClassType reg_class) const { return regClass == reg_class; }

 /** Index accessors */
 /** @{ */
@@ -202,7 +203,7 @@
 /** Elem accessor */
 RegIndex elemIndex() const { return elemIdx; }
 /** Class accessor */
-RegClass classValue() const { return regClass; }
+RegClassType classValue() const { return regClass; }
 /** Return a const char* with the register class name. */
 const char* className() const { return regClassStrings[regClass]; }

@@ -233,14 +234,14 @@
 {}

 /** Scalar PhysRegId constructor. */
-explicit PhysRegId(RegClass _regClass, RegIndex _regIdx,
+explicit PhysRegId(RegClassType _regClass, RegIndex _regIdx,
   RegIndex _flatIdx)
 : RegId(_regClass, _regIdx), flatIdx(_flatIdx),
   numPinnedWritesToComplete(0), pinned(false)
 {}

 /** Vector PhysRegId constructor (w/ elemIndex). */
-explicit PhysRegId(RegClass _regClass, RegIndex _regIdx,
+explicit PhysRegId(RegClassType _regClass, RegIndex _regIdx,
   ElemIndex elem_idx, RegIndex flat_idx)
 : RegId(_regClass, _regIdx, elem_idx), flatIdx(flat_idx),
   numPinnedWritesToComplete(0), pinned(false)

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

[gem5-dev] Change in gem5/gem5[develop]: base: handle initial communication with GDB in `attach()`

[Jan Vrany (Gerrit) via gem5-dev]

Jan Vrany has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/48182 )


Change subject: base: handle initial communication with GDB in `attach()`
..

base: handle initial communication with GDB in `attach()`

When remote GDB attaches to gem5, handle the initial communication
(`qSupported` and alike) right away instead of scheduling a `DataEvent`
and firing the simulation loop in hope that GDB will be quick enough to
send initial packets before instructions are dispatched.

This requires attach() to be always called at instruction boundary
to make it safe to interact with the rest of gem5.

When `--wait-gdb` is used, connect() is called from workflow startup,
therefore on an instruction boundary and therefore needs not special
handling.

To handle case the GDB connects while simulation is already running,
we arrange (new) assynchronous IncommingConnectionEvent on listening
socket that, when there's a new connection being made, *only* schedules
*synchronous* ConnectEvent that handles the rest, *including* calling
an accept() on listening socket. This way it is safe to process commands
in attach().

In order to make the code more systematic and easier to understands,
detach() is also made to be called only synchronously (that is, at
intruction boundary). Asynchronous events and event handlers are
prefixed with "incoming".

This seems to fix the race described in 44612 [1]

[1]: https://gem5-review.googlesource.com/c/public/gem5/+/44612

Change-Id: I33b2922ba017205acabd51b6a8be3e6fb2d6409a
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48182
Reviewed-by: Boris Shingarov 
Maintainer: Boris Shingarov 
Tested-by: kokoro 
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 79 insertions(+), 33 deletions(-)

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



diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index b53cbc0..1437b75 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -356,14 +356,16 @@
 }

 BaseRemoteGDB::BaseRemoteGDB(System *_system, int _port) :
-connectEvent(nullptr), dataEvent(nullptr), _port(_port), fd(-1),
-sys(_system), trapEvent(this), singleStepEvent(*this)
+incomingConnectionEvent(nullptr), incomingDataEvent(nullptr),
+_port(_port), fd(-1), sys(_system),
+connectEvent(this), disconnectEvent(this), trapEvent(this),
+singleStepEvent(*this)
 {}

 BaseRemoteGDB::~BaseRemoteGDB()
 {
-delete connectEvent;
-delete dataEvent;
+delete incomingConnectionEvent;
+delete incomingDataEvent;
 }

 std::string
@@ -385,8 +387,9 @@
 _port++;
 }

-connectEvent = new ConnectEvent(this, listener.getfd(), POLLIN);
-pollQueue.schedule(connectEvent);
+incomingConnectionEvent =
+new IncomingConnectionEvent(this, listener.getfd(), POLLIN);
+pollQueue.schedule(incomingConnectionEvent);

 ccprintf(std::cerr, "%d: %s: listening for remote gdb on port %d\n",
  curTick(), name(), _port);
@@ -398,6 +401,8 @@
 panic_if(!listener.islistening(),
  "Can't accept GDB connections without any threads!");

+pollQueue.remove(incomingConnectionEvent);
+
 int sfd = listener.accept(true);

 if (sfd != -1) {
@@ -421,23 +426,48 @@
 {
 fd = f;

-dataEvent = new DataEvent(this, fd, POLLIN);
-pollQueue.schedule(dataEvent);
-
 attached = true;
 DPRINTFN("remote gdb attached\n");
+
+processCommands();
+
+if (isAttached()) {
+// At this point an initial communication with GDB is handled
+// and we're ready to continue. Here we arrange IncomingDataEvent
+// to get notified when GDB breaks in.
+//
+// However, GDB can decide to disconnect during that initial
+// communication. In that case, we cannot arrange data event  
because
+// the socket is already closed (not that it makes any sense,  
anyways).

+//
+// Hence the check above.
+incomingDataEvent = new IncomingDataEvent(this, fd, POLLIN);
+pollQueue.schedule(incomingDataEvent);
+}
 }

 void
 BaseRemoteGDB::detach()
 {
 attached = false;
-active = false;
 clearSingleStep();
 close(fd);
 fd = -1;

-pollQueue.remove(dataEvent);
+if (incomingDataEvent) {
+// incomingDataEvent gets scheduled in attach() after
+// initial communication with GDB is handled and GDB tells
+// gem5 to continue.
+//
+// GDB can disconnect before that in which case `incomingDataEvent`
+// is NULL.
+//
+// Hence the check above.
+
+pollQueue.remove(incomingDataEvent);
+incomingDataEvent = nullptr;
+}
+pollQueue.schedule(incomingConnectionEvent);
 DPRINTFN("remote gdb detached\n");
 }

@@ -498,19 +528,7 @@

 

[gem5-dev] Change in gem5/gem5[develop]: base: Extract GDB command processing into separate function

[Jan Vrany (Gerrit) via gem5-dev]

Jan Vrany has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/48181 )


Change subject: base: Extract GDB command processing into separate function
..

base: Extract GDB command processing into separate function

Change-Id: I1f090285f92752d6907044b9ee6ade1869a2cb9f
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48181
Reviewed-by: Gabe Black 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/base/remote_gdb.cc
M src/base/remote_gdb.hh
2 files changed, 58 insertions(+), 44 deletions(-)

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



diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index ad95046..b53cbc0 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -519,50 +519,7 @@
 send("S%02x", signum);
 }

-// Stick frame regs into our reg cache.
-regCachePtr = gdbRegs();
-regCachePtr->getRegs(tc);
-
-GdbCommand::Context cmd_ctx;
-cmd_ctx.type = signum;
-std::vector data;
-
-for (;;) {
-try {
-recv(data);
-if (data.size() == 1)
-throw BadClient();
-cmd_ctx.cmdByte = data[0];
-cmd_ctx.data = data.data() + 1;
-// One for sentinel, one for cmdByte.
-cmd_ctx.len = data.size() - 2;
-
-auto cmd_it = commandMap.find(cmd_ctx.cmdByte);
-if (cmd_it == commandMap.end()) {
-DPRINTF(GDBMisc, "Unknown command: %c(%#x)\n",
-cmd_ctx.cmdByte, cmd_ctx.cmdByte);
-throw Unsupported();
-}
-cmd_ctx.cmd = &(cmd_it->second);
-
-if (!(this->*(cmd_ctx.cmd->func))(cmd_ctx))
-break;
-
-} catch (BadClient ) {
-if (e.warning)
-warn(e.warning);
-detach();
-break;
-} catch (Unsupported ) {
-send("");
-} catch (CmdError ) {
-send(e.error);
-} catch (std::exception ) {
-panic("Unrecognzied GDB exception: %s", e.what());
-} catch (...) {
-panic("Unrecognzied GDB exception.");
-}
-}
+processCommands(signum);
 }

 void
@@ -683,6 +640,55 @@
 } while ((c & 0x7f) == GDBBadP);
 }

+void
+BaseRemoteGDB::processCommands(int signum)
+{
+// Stick frame regs into our reg cache.
+regCachePtr = gdbRegs();
+regCachePtr->getRegs(tc);
+
+GdbCommand::Context cmd_ctx;
+cmd_ctx.type = signum;
+std::vector data;
+
+for (;;) {
+try {
+recv(data);
+if (data.size() == 1)
+throw BadClient();
+cmd_ctx.cmdByte = data[0];
+cmd_ctx.data = data.data() + 1;
+// One for sentinel, one for cmdByte.
+cmd_ctx.len = data.size() - 2;
+
+auto cmd_it = commandMap.find(cmd_ctx.cmdByte);
+if (cmd_it == commandMap.end()) {
+DPRINTF(GDBMisc, "Unknown command: %c(%#x)\n",
+cmd_ctx.cmdByte, cmd_ctx.cmdByte);
+throw Unsupported();
+}
+cmd_ctx.cmd = &(cmd_it->second);
+
+if (!(this->*(cmd_ctx.cmd->func))(cmd_ctx))
+break;
+
+} catch (BadClient ) {
+if (e.warning)
+warn(e.warning);
+detach();
+break;
+} catch (Unsupported ) {
+send("");
+} catch (CmdError ) {
+send(e.error);
+} catch (std::exception ) {
+panic("Unrecognized GDB exception: %s", e.what());
+} catch (...) {
+panic("Unrecognized GDB exception.");
+}
+}
+}
+
 // Read bytes from kernel address space for debugger.
 bool
 BaseRemoteGDB::read(Addr vaddr, size_t size, char *data)
diff --git a/src/base/remote_gdb.hh b/src/base/remote_gdb.hh
index 7d8305b..f04bc9d 100644
--- a/src/base/remote_gdb.hh
+++ b/src/base/remote_gdb.hh
@@ -238,6 +238,14 @@
 }

 /*
+ * Process commands from remote GDB. If simulation has been
+ * stopped because of some kind of fault (as segmentation violation,
+ * or SW trap), 'signum' is the signal value reported back to GDB
+ * in "S" packet (this is done in trap()).
+ */
+void processCommands(int signum=0);
+
+/*
  * Simulator side debugger state.
  */
 bool active = false;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48181
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: I1f090285f92752d6907044b9ee6ade1869a2cb9f
Gerrit-Change-Number: 48181
Gerrit-PatchSet: 5
Gerrit-Owner: Jan Vrany 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Boris Shingarov 
Gerrit-Reviewer: Ciro Santilli 

[gem5-dev] Re: Build failed in Jenkins: compiler-checks #145

[Gabe Black via gem5-dev]

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

On Mon, Aug 2, 2021 at 11:19 AM Jason Lowe-Power via gem5-dev <
gem5-dev@gem5.org> wrote:

> Looks like GCN is failing with clang-11. Could someone take a look? Bobby
> is on vacation this week.
>
> Cheers,
> Jason
>
> On Mon, Aug 2, 2021 at 12:07 AM jenkins-no-reply--- via gem5-dev <
> gem5-dev@gem5.org> wrote:
>
>> See <
>> https://jenkins.gem5.org/job/compiler-checks/145/display/redirect?page=changes
>> >
>>
>> Changes:
>>
>> [gabe.black] dev: Fix style in i8254xGBe_defs.hh.
>>
>> [gabe.black] dev: Fix style in pktfifo.hh.
>>
>> [gabe.black] dev: Fix style in i8254xGBE.cc.
>>
>>
>> --
>> Started by timer
>> Running as SYSTEM
>> Building in workspace 
>> Selected Git installation does not exist. Using Default
>> The recommended git tool is: NONE
>> No credentials specified
>>  > git rev-parse --resolve-git-dir <
>> https://jenkins.gem5.org/job/compiler-checks/ws/.git> # timeout=10
>> Fetching changes from the remote Git repository
>>  > git config remote.origin.url https://gem5.googlesource.com/public/gem5
>> # timeout=10
>> Fetching upstream changes from https://gem5.googlesource.com/public/gem5
>>  > git --version # timeout=10
>>  > git --version # 'git version 2.25.1'
>>  > git fetch --tags --force --progress --
>> https://gem5.googlesource.com/public/gem5
>> +refs/heads/*:refs/remotes/origin/* # timeout=10
>>  > git rev-parse refs/remotes/origin/develop^{commit} # timeout=10
>> Checking out Revision e24db5deddeed1a30c0a635b271b4d364a54c44d
>> (refs/remotes/origin/develop)
>>  > git config core.sparsecheckout # timeout=10
>>  > git checkout -f e24db5deddeed1a30c0a635b271b4d364a54c44d # timeout=10
>> Commit message: "dev: Fix style in i8254xGBE.cc."
>>  > git rev-list --no-walk 2cde260198b395177040b48b81fe14615aeaae8c #
>> timeout=10
>> [compiler-checks] $ /bin/sh -xe /tmp/jenkins5372034535609594944.sh
>> + ./tests/compiler-tests.sh -j 12
>> Starting build tests with 'gcc-version-10'...
>> 'gcc-version-10' was found in the comprehensive tests. All ISAs will be
>> built.
>>   * Building target 'ARM_MESI_Three_Level.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'ARM_MESI_Three_Level.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'SPARC.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'SPARC.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'NULL_MOESI_CMP_token.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'NULL_MOESI_CMP_token.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'ARM_MOESI_hammer.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'ARM_MOESI_hammer.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'POWER.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'POWER.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'ARM_MESI_Three_Level_HTM.opt' with
>> 'gcc-version-10'...
>> Done.
>>   * Building target 'ARM_MESI_Three_Level_HTM.fast' with
>> 'gcc-version-10'...
>> Done.
>>   * Building target 'NULL_MESI_Two_Level.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'NULL_MESI_Two_Level.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'X86.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'X86.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'NULL.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'NULL.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'Garnet_standalone.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'Garnet_standalone.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'RISCV.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'RISCV.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'NULL_MOESI_CMP_directory.opt' with
>> 'gcc-version-10'...
>> Done.
>>   * Building target 'NULL_MOESI_CMP_directory.fast' with
>> 'gcc-version-10'...
>> Done.
>>   * Building target 'X86_MESI_Two_Level.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'X86_MESI_Two_Level.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'X86_MOESI_AMD_Base.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'X86_MOESI_AMD_Base.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'NULL_MOESI_hammer.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'NULL_MOESI_hammer.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'GCN3_X86.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'GCN3_X86.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'ARM.opt' with 'gcc-version-10'...
>> Done.
>>   * Building target 'ARM.fast' with 'gcc-version-10'...
>> Done.
>>   * Building target 'MIPS.opt' with 

[gem5-dev] Change in gem5/gem5[develop]: arch-gcn3: Fix initAtomicAccess.

[Gabe Black (Gerrit) via gem5-dev]

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



Change subject: arch-gcn3: Fix initAtomicAccess.
..

arch-gcn3: Fix initAtomicAccess.

This function used makeAtomicOpFunctor to create a unique_ptr which
pointed to an AtomicOpFunctor *, which it immediately extracted with
.get(). Then since the temporary unique_ptr went out of scope, it
deleted the AtomicOpFunctor which it just returned a pointer to.

Instead, that function should create a local unique_ptr to pass
ownership of the object off to. It will still be cleaned up when it
goes out of scope, but not before it's done being used.

Change-Id: I74a0bcbb719a78a3e9ec8cb2ea5aa15120da0456
---
M src/arch/amdgpu/gcn3/insts/op_encodings.hh
1 file changed, 2 insertions(+), 2 deletions(-)



diff --git a/src/arch/amdgpu/gcn3/insts/op_encodings.hh  
b/src/arch/amdgpu/gcn3/insts/op_encodings.hh

index 27b9b99..24edfa7 100644
--- a/src/arch/amdgpu/gcn3/insts/op_encodings.hh
+++ b/src/arch/amdgpu/gcn3/insts/op_encodings.hh
@@ -886,12 +886,12 @@
 for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) {
 if (gpuDynInst->exec_mask[lane]) {
 Addr vaddr = gpuDynInst->addr[lane];
-AtomicOpFunctor* amo_op =
+auto amo_op =
 gpuDynInst->makeAtomicOpFunctor(
 &(reinterpret_cast(
 gpuDynInst->a_data))[lane],
 &(reinterpret_cast(
-gpuDynInst->x_data))[lane]).get();
+gpuDynInst->x_data))[lane]);

 T tmp = wf->ldsChunk->read(vaddr);
 (*amo_op)(reinterpret_cast());

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49023
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: I74a0bcbb719a78a3e9ec8cb2ea5aa15120da0456
Gerrit-Change-Number: 49023
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] Build failed in Jenkins: nightly #406

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

See 

Changes:

[Giacomo Travaglini] base: Add an exclude method to the AddrRange class

[Giacomo Travaglini] python: Expose the AddrRange exclude to the python world

[gabe.black] arm: Make the misc reg class return the name of misc regs.

[gabe.black] cpu-minor: Use the RegClassInfo::regName method instead of THE_ISA.

[gabe.black] misc: Replace THE_ISA macro with IS_NULL_ISA.

[daecheol.you] mem-ruby: Atomic transaction support for CHI protocol

[binarystar.kai.ren] configs: Add --wait-gdb option to fs.py


--
[...truncated 972.78 KB...]
 [LINK]  -> NULL/base/channel_addr.test.perf
build/NULL/base/channel_addr.test.perf 
--gtest_output=xml:build/NULL/unittests.perf/base/channel_addr.test.xml
Running main() from build/googletest/googletest/src/gtest_main.cc
[==] Running 2 tests from 1 test suite.
[--] Global test environment set-up.
[--] 2 tests from ChannelAddrRange
[ RUN  ] ChannelAddrRange.DefaultInvalid
[   OK ] ChannelAddrRange.DefaultInvalid (0 ms)
[ RUN  ] ChannelAddrRange.Range
[   OK ] ChannelAddrRange.Range (0 ms)
[--] 2 tests from ChannelAddrRange (0 ms total)

[--] Global test environment tear-down
[==] 2 tests from 1 test suite ran. (0 ms total)
[  PASSED  ] 2 tests.
build/NULL/base/pixel.test.perf 
--gtest_output=xml:build/NULL/unittests.perf/base/pixel.test.xml
Running main() from build/googletest/googletest/src/gtest_main.cc
[==] Running 5 tests from 1 test suite.
[--] Global test environment set-up.
[--] 5 tests from FBTest
[ RUN  ] FBTest.PixelConversionRGBA
[   OK ] FBTest.PixelConversionRGBA (0 ms)
[ RUN  ] FBTest.PixelConversionRGB565
[   OK ] FBTest.PixelConversionRGB565 (0 ms)
[ RUN  ] FBTest.PixelToMemRGBALE
[   OK ] FBTest.PixelToMemRGBALE (0 ms)
[ RUN  ] FBTest.MemToPixelRGBALE
[   OK ] FBTest.MemToPixelRGBALE (0 ms)
[ RUN  ] FBTest.MemToPixelRGBABE
[   OK ] FBTest.MemToPixelRGBABE (0 ms)
[--] 5 tests from FBTest (0 ms total)

[--] Global test environment tear-down
[==] 5 tests from 1 test suite ran. (0 ms total)
[  PASSED  ] 5 tests.
build/NULL/sim/byteswap.test.perf 
--gtest_output=xml:build/NULL/unittests.perf/sim/byteswap.test.xml
Running main() from build/googletest/googletest/src/gtest_main.cc
[==] Running 8 tests from 1 test suite.
[--] Global test environment set-up.
[--] 8 tests from ByteswapTest
[ RUN  ] ByteswapTest.swap_byte64
[   OK ] ByteswapTest.swap_byte64 (0 ms)
[ RUN  ] ByteswapTest.swap_byte32
[   OK ] ByteswapTest.swap_byte32 (0 ms)
[ RUN  ] ByteswapTest.swap_byte16
[   OK ] ByteswapTest.swap_byte16 (0 ms)
[ RUN  ] ByteswapTest.swap_byte
[   OK ] ByteswapTest.swap_byte (0 ms)
[ RUN  ] ByteswapTest.htog
[   OK ] ByteswapTest.htog (0 ms)
[ RUN  ] ByteswapTest.gtoh
[   OK ] ByteswapTest.gtoh (0 ms)
[ RUN  ] ByteswapTest.betole
[   OK ] ByteswapTest.betole (0 ms)
[ RUN  ] ByteswapTest.letobe
[   OK ] ByteswapTest.letobe (0 ms)
[--] 8 tests from ByteswapTest (0 ms total)

[--] Global test environment tear-down
[==] 8 tests from 1 test suite ran. (0 ms total)
[  PASSED  ] 8 tests.
 [LINK]  -> NULL/base/addr_range_map.test.perf
build/NULL/base/addr_range_map.test.perf 
--gtest_output=xml:build/NULL/unittests.perf/base/addr_range_map.test.xml
Running main() from build/googletest/googletest/src/gtest_main.cc
[==] Running 3 tests from 1 test suite.
[--] Global test environment set-up.
[--] 3 tests from AddrRangeMapTest
[ RUN  ] AddrRangeMapTest.LegacyTests
[   OK ] AddrRangeMapTest.LegacyTests (0 ms)
[ RUN  ] AddrRangeMapTest.InterleavedTest1
[   OK ] AddrRangeMapTest.InterleavedTest1 (0 ms)
[ RUN  ] AddrRangeMapTest.InterleavedTest2
[   OK ] AddrRangeMapTest.InterleavedTest2 (0 ms)
[--] 3 tests from AddrRangeMapTest (0 ms total)

[--] Global test environment tear-down
[==] 3 tests from 1 test suite ran. (0 ms total)
[  PASSED  ] 3 tests.
 [LINK]  -> NULL/base/addr_range.test.perf
build/NULL/base/addr_range.test.perf 
--gtest_output=xml:build/NULL/unittests.perf/base/addr_range.test.xml
Running main() from build/googletest/googletest/src/gtest_main.cc
[==] Running 66 tests from 2 test suites.
[--] Global test environment set-up.
[--] 1 test from AddrRangeDeathTest
[ RUN  ] AddrRangeDeathTest.ExcludeInterleavingRanges
build/NULL/base/addr_range.test.cc:1426: Failure
Death test: r.exclude(exclude_ranges)
Result: threw an exception.
 Error msg:
[  DEATH   ] build/NULL/base/addr_range.hh:399: panic: Cannot test intersection 
of [0x100:0x200] a[0]^=0 and [0x180:0x210]
[  DEATH   ] 
[  FAILED  ] AddrRangeDeathTest.ExcludeInterleavingRanges (1 ms)
[--] 

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Explicitly implement I/DTLBI Ops in TLB

[Giacomo Travaglini (Gerrit) via gem5-dev]

Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/48148 )


Change subject: arch-arm: Explicitly implement I/DTLBI Ops in TLB
..

arch-arm: Explicitly implement I/DTLBI Ops in TLB

At the moment the ITLBI and DTLBI operations where implicitly
implemented as generic TLBIs

e.g:

* DTLBIASID, ITLBIASID = TLBIASID

This was possible as a single TLB was either an instruction TLB
or a data TLB and no generic/shared TLB option was available.
In other words, an ITLBI Op to an ITB was invalidating all entries
as we were sure the ITB was not containing data entries.

With shared TLBs, this doesn't hold true and we need to explicitly
implement I/DTLBIs

JIRA: https://gem5.atlassian.net/browse/GEM5-790

Change-Id: I39a4add7674f6008dacaedfd1fd90560d264048e
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48148
Reviewed-by: Andreas Sandberg 
Maintainer: Andreas Sandberg 
Tested-by: kokoro 
---
M src/arch/arm/tlb.cc
M src/arch/arm/tlb.hh
2 files changed, 145 insertions(+), 5 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/tlb.cc b/src/arch/arm/tlb.cc
index dc0e155..666836b 100644
--- a/src/arch/arm/tlb.cc
+++ b/src/arch/arm/tlb.cc
@@ -229,6 +229,56 @@
 }

 void
+TLB::flush(const ITLBIALL& tlbi_op)
+{
+DPRINTF(TLB, "Flushing all ITLB entries (%s lookup)\n",
+(tlbi_op.secureLookup ? "secure" : "non-secure"));
+int x = 0;
+TlbEntry *te;
+while (x < size) {
+te = [x];
+const bool el_match = te->checkELMatch(
+tlbi_op.targetEL, tlbi_op.inHost);
+if (te->type & TypeTLB::instruction && te->valid &&
+tlbi_op.secureLookup == !te->nstid &&
+(te->vmid == vmid || tlbi_op.el2Enabled) && el_match) {
+
+DPRINTF(TLB, " -  %s\n", te->print());
+te->valid = false;
+stats.flushedEntries++;
+}
+++x;
+}
+
+stats.flushTlb++;
+}
+
+void
+TLB::flush(const DTLBIALL& tlbi_op)
+{
+DPRINTF(TLB, "Flushing all DTLB entries (%s lookup)\n",
+(tlbi_op.secureLookup ? "secure" : "non-secure"));
+int x = 0;
+TlbEntry *te;
+while (x < size) {
+te = [x];
+const bool el_match = te->checkELMatch(
+tlbi_op.targetEL, tlbi_op.inHost);
+if (te->type & TypeTLB::data && te->valid &&
+tlbi_op.secureLookup == !te->nstid &&
+(te->vmid == vmid || tlbi_op.el2Enabled) && el_match) {
+
+DPRINTF(TLB, " -  %s\n", te->print());
+te->valid = false;
+stats.flushedEntries++;
+}
+++x;
+}
+
+stats.flushTlb++;
+}
+
+void
 TLB::flush(const TLBIALLEL _op)
 {
 DPRINTF(TLB, "Flushing all TLB entries (%s lookup)\n",
@@ -307,7 +357,29 @@
 "(%s lookup)\n", tlbi_op.addr, tlbi_op.asid,
 (tlbi_op.secureLookup ? "secure" : "non-secure"));
 _flushMva(tlbi_op.addr, tlbi_op.asid, tlbi_op.secureLookup, false,
-tlbi_op.targetEL, tlbi_op.inHost);
+tlbi_op.targetEL, tlbi_op.inHost, TypeTLB::unified);
+stats.flushTlbMvaAsid++;
+}
+
+void
+TLB::flush(const ITLBIMVA _op)
+{
+DPRINTF(TLB, "Flushing ITLB entries with mva: %#x, asid: %#x "
+"(%s lookup)\n", tlbi_op.addr, tlbi_op.asid,
+(tlbi_op.secureLookup ? "secure" : "non-secure"));
+_flushMva(tlbi_op.addr, tlbi_op.asid, tlbi_op.secureLookup, false,
+tlbi_op.targetEL, tlbi_op.inHost, TypeTLB::instruction);
+stats.flushTlbMvaAsid++;
+}
+
+void
+TLB::flush(const DTLBIMVA _op)
+{
+DPRINTF(TLB, "Flushing DTLB entries with mva: %#x, asid: %#x "
+"(%s lookup)\n", tlbi_op.addr, tlbi_op.asid,
+(tlbi_op.secureLookup ? "secure" : "non-secure"));
+_flushMva(tlbi_op.addr, tlbi_op.asid, tlbi_op.secureLookup, false,
+tlbi_op.targetEL, tlbi_op.inHost, TypeTLB::data);
 stats.flushTlbMvaAsid++;
 }

@@ -337,19 +409,72 @@
 }

 void
+TLB::flush(const ITLBIASID _op)
+{
+DPRINTF(TLB, "Flushing ITLB entries with asid: %#x (%s lookup)\n",
+tlbi_op.asid,  
(tlbi_op.secureLookup ? "secure" : "non-secure"));

+
+int x = 0 ;
+TlbEntry *te;
+
+while (x < size) {
+te = [x];
+if (te->type & TypeTLB::instruction &&
+te->valid && te->asid == tlbi_op.asid &&
+tlbi_op.secureLookup == !te->nstid &&
+(te->vmid == vmid || tlbi_op.el2Enabled) &&
+te->checkELMatch(tlbi_op.targetEL, tlbi_op.inHost)) {
+
+te->valid = false;
+DPRINTF(TLB, " -  %s\n", te->print());
+stats.flushedEntries++;
+}
+++x;
+}
+stats.flushTlbAsid++;
+}
+
+void
+TLB::flush(const DTLBIASID _op)
+{
+DPRINTF(TLB, "Flushing DTLB entries with asid: %#x (%s lookup)\n",

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Provide support for a multilevel-TLB in the ArmMMU

[Giacomo Travaglini (Gerrit) via gem5-dev]

Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/48149 )


Change subject: arch-arm: Provide support for a multilevel-TLB in the ArmMMU
..

arch-arm: Provide support for a multilevel-TLB in the ArmMMU

This is an initial implementation. It adapts the current MMU code
to account for extra levels of TLBs but it is still missing the
configurability we are looking for (to select the associativity
of the TLB and the replacement policy as an example)

JIRA: https://gem5.atlassian.net/browse/GEM5-790

Change-Id: I938ec38183337cd0e839bf3e3cd03594126128cd
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48149
Tested-by: kokoro 
Reviewed-by: Andreas Sandberg 
Maintainer: Andreas Sandberg 
---
M src/arch/arm/mmu.cc
M src/arch/arm/mmu.hh
M src/arch/arm/table_walker.cc
M src/arch/arm/tlb.cc
M src/arch/arm/tlb.hh
5 files changed, 148 insertions(+), 22 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/mmu.cc b/src/arch/arm/mmu.cc
index 257b3e6..d2fc706 100644
--- a/src/arch/arm/mmu.cc
+++ b/src/arch/arm/mmu.cc
@@ -1089,8 +1089,16 @@

 itbStage2->setVMID(state.vmid);
 dtbStage2->setVMID(state.vmid);
-getITBPtr()->setVMID(state.vmid);
-getDTBPtr()->setVMID(state.vmid);
+
+for (auto tlb : instruction) {
+static_cast(tlb)->setVMID(state.vmid);
+}
+for (auto tlb : data) {
+static_cast(tlb)->setVMID(state.vmid);
+}
+for (auto tlb : unified) {
+static_cast(tlb)->setVMID(state.vmid);
+}

 miscRegContext = tc->contextId();
 }
@@ -1309,6 +1317,16 @@
 is_secure, tran_type, stage2 ? s2State : s1State);
 }

+TlbEntry*
+MMU::lookup(Addr va, uint16_t asid, vmid_t vmid, bool hyp, bool secure,
+bool functional, bool ignore_asn, ExceptionLevel target_el,
+bool in_host, bool stage2, BaseMMU::Mode mode)
+{
+TLB *tlb = getTlb(mode, stage2);
+return tlb->multiLookup(va, asid, vmid, hyp, secure, functional,
+ignore_asn, target_el, in_host, mode);
+}
+
 Fault
 MMU::getTE(TlbEntry **te, const RequestPtr , ThreadContext *tc, Mode  
mode,

 Translation *translation, bool timing, bool functional,
@@ -1331,9 +1349,9 @@
 vaddr = vaddr_tainted;
 }

-auto tlb = getTlb(mode, state.isStage2);
-*te = tlb->lookup(vaddr, state.asid, state.vmid, state.isHyp,  
is_secure,

-  false, false, target_el, false, mode);
+*te = lookup(vaddr, state.asid, state.vmid, state.isHyp, is_secure,  
false,

+ false, target_el, false, state.isStage2, mode);
+
 if (*te == NULL) {
 if (req->isPrefetch()) {
 // if the request is a prefetch don't attempt to fill the TLB  
or go

@@ -1361,10 +1379,8 @@
 return fault;
 }

-*te = tlb->lookup(vaddr, state.asid, state.vmid, state.isHyp,  
is_secure,

-  true, false, target_el, false, mode);
-if (!*te)
-tlb->printTlb();
+*te = lookup(vaddr, state.asid, state.vmid, state.isHyp, is_secure,
+ true, false, target_el, false, state.isStage2, mode);
 assert(*te);
 }
 return NoFault;
diff --git a/src/arch/arm/mmu.hh b/src/arch/arm/mmu.hh
index fddaa1f..0e1fd87 100644
--- a/src/arch/arm/mmu.hh
+++ b/src/arch/arm/mmu.hh
@@ -269,8 +269,15 @@
 void
 flushStage1(const OP _op)
 {
-iflush(tlbi_op);
-dflush(tlbi_op);
+for (auto tlb : instruction) {
+static_cast(tlb)->flush(tlbi_op);
+}
+for (auto tlb : data) {
+static_cast(tlb)->flush(tlbi_op);
+}
+for (auto tlb : unified) {
+static_cast(tlb)->flush(tlbi_op);
+}
 }

 template 
@@ -285,14 +292,24 @@
 void
 iflush(const OP _op)
 {
-getITBPtr()->flush(tlbi_op);
+for (auto tlb : instruction) {
+static_cast(tlb)->flush(tlbi_op);
+}
+for (auto tlb : unified) {
+static_cast(tlb)->flush(tlbi_op);
+}
 }

 template 
 void
 dflush(const OP _op)
 {
-getDTBPtr()->flush(tlbi_op);
+for (auto tlb : data) {
+static_cast(tlb)->flush(tlbi_op);
+}
+for (auto tlb : unified) {
+static_cast(tlb)->flush(tlbi_op);
+}
 }

 void
@@ -325,6 +342,24 @@
 static ExceptionLevel tranTypeEL(CPSR cpsr, ArmTranslationType type);

   public:
+/** Lookup an entry in the TLB
+ * @param vpn virtual address
+ * @param asn context id/address space id to use
+ * @param vmid The virtual machine ID used for stage 2 translation
+ * @param secure if the lookup is secure

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Distinguish Instruction from Data TLB entries

[Giacomo Travaglini (Gerrit) via gem5-dev]

Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/48147 )


Change subject: arch-arm: Distinguish Instruction from Data TLB entries
..

arch-arm: Distinguish Instruction from Data TLB entries

As we are going to support I+D (shared) TLBs we need to tag a TLB entry
as a data or instruction entry, so that I-TLBI and D-TLBI do not
over-invalidate entries

(The patch is also fixing the coding style of the TLB insertion
methods in the Table Walker)

JIRA: https://gem5.atlassian.net/browse/GEM5-790

Change-Id: I3d5880175fe6eda1b2f0edcbd0ea6a146d3c7a39
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48147
Tested-by: kokoro 
Reviewed-by: Andreas Sandberg 
Maintainer: Andreas Sandberg 
---
M src/arch/arm/pagetable.hh
M src/arch/arm/table_walker.cc
2 files changed, 30 insertions(+), 19 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/pagetable.hh b/src/arch/arm/pagetable.hh
index 53780d7..c3d8738 100644
--- a/src/arch/arm/pagetable.hh
+++ b/src/arch/arm/pagetable.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2012-2013 ARM Limited
+ * Copyright (c) 2010, 2012-2013, 2021 Arm Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -46,6 +46,7 @@
 #include "arch/arm/page_size.hh"
 #include "arch/arm/types.hh"
 #include "arch/arm/utility.hh"
+#include "enums/TypeTLB.hh"
 #include "sim/serialize.hh"

 namespace gem5
@@ -136,6 +137,9 @@
 bool nstid;
 // Exception level on insert, AARCH64 EL0&1, AARCH32 -> el=1
 ExceptionLevel el;
+// This is used to distinguish between instruction and data entries
+// in unified TLBs
+TypeTLB type;

 // Type of memory
 bool nonCacheable; // Can we wrap this in mtype?
@@ -156,7 +160,8 @@
  innerAttrs(0), outerAttrs(0), ap(read_only ? 0x3 : 0), hap(0x3),
  domain(DomainType::Client),  mtype(MemoryType::StronglyOrdered),
  longDescFormat(false), isHyp(false), global(false), valid(true),
- ns(true), nstid(true), el(EL0), nonCacheable(uncacheable),
+ ns(true), nstid(true), el(EL0), type(TypeTLB::unified),
+ nonCacheable(uncacheable),
  shareable(false), outerShareable(false), xn(0), pxn(0)
 {
 // no restrictions by default, hap = 0x3
@@ -171,7 +176,8 @@
  vmid(0), N(0), innerAttrs(0), outerAttrs(0), ap(0), hap(0x3),
  domain(DomainType::Client), mtype(MemoryType::StronglyOrdered),
  longDescFormat(false), isHyp(false), global(false), valid(false),
- ns(true), nstid(true), el(EL0), nonCacheable(false),
+ ns(true), nstid(true), el(EL0), type(TypeTLB::unified),
+ nonCacheable(false),
  shareable(false), outerShareable(false), xn(0), pxn(0)
 {
 // no restrictions by default, hap = 0x3
@@ -317,6 +323,7 @@
 SERIALIZE_SCALAR(valid);
 SERIALIZE_SCALAR(ns);
 SERIALIZE_SCALAR(nstid);
+SERIALIZE_ENUM(type);
 SERIALIZE_SCALAR(nonCacheable);
 SERIALIZE_ENUM(lookupLevel);
 SERIALIZE_ENUM(mtype);
@@ -347,6 +354,7 @@
 UNSERIALIZE_SCALAR(valid);
 UNSERIALIZE_SCALAR(ns);
 UNSERIALIZE_SCALAR(nstid);
+UNSERIALIZE_ENUM(type);
 UNSERIALIZE_SCALAR(nonCacheable);
 UNSERIALIZE_ENUM(lookupLevel);
 UNSERIALIZE_ENUM(mtype);
diff --git a/src/arch/arm/table_walker.cc b/src/arch/arm/table_walker.cc
index dfdf0ad..2d1cfa6 100644
--- a/src/arch/arm/table_walker.cc
+++ b/src/arch/arm/table_walker.cc
@@ -1449,16 +1449,16 @@

 void
 TableWalker::memAttrsLPAE(ThreadContext *tc, TlbEntry ,
-LongDescriptor )
+LongDescriptor _descriptor)
 {
 assert(_haveLPAE);

 uint8_t attr;
-uint8_t sh = lDescriptor.sh();
+uint8_t sh = l_descriptor.sh();
 // Different format and source of attributes if this is a stage 2
 // translation
 if (isStage2) {
-attr = lDescriptor.memAttr();
+attr = l_descriptor.memAttr();
 uint8_t attr_3_2 = (attr >> 2) & 0x3;
 uint8_t attr_1_0 =  attr   & 0x3;

@@ -1479,7 +1479,7 @@
 te.nonCacheable = (attr_3_2 == 1) || (attr_1_0 == 1);
 }
 } else {
-uint8_t attrIndx = lDescriptor.attrIndx();
+uint8_t attrIndx = l_descriptor.attrIndx();

 // LPAE always uses remapping of memory attributes, irrespective  
of the

 // value of SCTLR.TRE
@@ -1575,15 +1575,15 @@

 void
 TableWalker::memAttrsAArch64(ThreadContext *tc, TlbEntry ,
- LongDescriptor )
+ LongDescriptor _descriptor)
 {
 uint8_t attr;
 uint8_t attr_hi;
 uint8_t attr_lo;
-uint8_t sh = lDescriptor.sh();
+uint8_t sh = l_descriptor.sh();

 if (isStage2) {

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Add a shared L2 TLB to the default ArmMMU

[Giacomo Travaglini (Gerrit) via gem5-dev]

Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/48150 )


Change subject: arch-arm: Add a shared L2 TLB to the default ArmMMU
..

arch-arm: Add a shared L2 TLB to the default ArmMMU

JIRA: https://gem5.atlassian.net/browse/GEM5-790

Change-Id: I542c287a99c8b277afb4cd939c09521798dcf2f8
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48150
Tested-by: kokoro 
Reviewed-by: Andreas Sandberg 
Maintainer: Andreas Sandberg 
---
M src/arch/arm/ArmMMU.py
1 file changed, 6 insertions(+), 2 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/ArmMMU.py b/src/arch/arm/ArmMMU.py
index 6f54373..29acbbc 100644
--- a/src/arch/arm/ArmMMU.py
+++ b/src/arch/arm/ArmMMU.py
@@ -63,8 +63,12 @@
 cxx_class = 'gem5::ArmISA::MMU'
 cxx_header = 'arch/arm/mmu.hh'

-itb = ArmTLB(entry_type="instruction")
-dtb = ArmTLB(entry_type="data")
+# L2 TLBs
+l2_shared = ArmTLB(entry_type="unified", size=1280)
+
+# L1 TLBs
+itb = ArmTLB(entry_type="instruction", next_level=Parent.l2_shared)
+dtb = ArmTLB(entry_type="data", next_level=Parent.l2_shared)

 sys = Param.System(Parent.any, "system object parameter")




8 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/+/48150
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: I542c287a99c8b277afb4cd939c09521798dcf2f8
Gerrit-Change-Number: 48150
Gerrit-PatchSet: 13
Gerrit-Owner: Giacomo Travaglini 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch: Implement operator& for TypeTLB

[Giacomo Travaglini (Gerrit) via gem5-dev]

Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/48463 )


Change subject: arch: Implement operator& for TypeTLB
..

arch: Implement operator& for TypeTLB

Change-Id: I05af52ba5e0ef84510ca3f4c27d8f9cd55e07d90
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48463
Reviewed-by: Andreas Sandberg 
Maintainer: Andreas Sandberg 
Tested-by: kokoro 
---
M src/arch/generic/BaseTLB.py
M src/arch/generic/tlb.hh
2 files changed, 24 insertions(+), 0 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/generic/BaseTLB.py b/src/arch/generic/BaseTLB.py
index 8af4585..cbc296b 100644
--- a/src/arch/generic/BaseTLB.py
+++ b/src/arch/generic/BaseTLB.py
@@ -45,6 +45,13 @@
 instruction: TLB contains instruction entries only
 data: TLB contains data entries only
 unified: TLB contains both instruction and data entries
+
+The enum values have been selected in order to perform bitwise
+operations on them. For example a unified TLB contains both
+instruction and data entries so code trying to assess if the
+TLB is storing (e.g.) data entries can do that with:
+
+bool has_data = tlb->type() & TypeTLB::data;
 """
 map = {
 'instruction' : 0x1,
diff --git a/src/arch/generic/tlb.hh b/src/arch/generic/tlb.hh
index 8f192eb..16b7eec 100644
--- a/src/arch/generic/tlb.hh
+++ b/src/arch/generic/tlb.hh
@@ -41,6 +41,8 @@
 #ifndef __ARCH_GENERIC_TLB_HH__
 #define __ARCH_GENERIC_TLB_HH__

+#include 
+
 #include "arch/generic/mmu.hh"
 #include "base/logging.hh"
 #include "enums/TypeTLB.hh"
@@ -125,6 +127,21 @@
 BaseTLB* nextLevel() const { return _nextLevel; }
 };

+/** Implementing the "&" bitwise operator for TypeTLB allows us to handle
+ * TypeTLB::unified efficiently. For example if I want to check if a TLB
+ * is storing instruction entries I can do this with:
+ *
+ * tlb->type() & TypeTLB::instruction
+ *
+ * which will cover both TypeTLB::instruction and TypeTLB::unified TLBs
+ */
+inline auto
+operator&(TypeTLB lhs, TypeTLB rhs)
+{
+using T = std::underlying_type_t;
+return static_cast(lhs) & static_cast(rhs);
+}
+
 } // namespace gem5

 #endif // __ARCH_GENERIC_TLB_HH__

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48463
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: I05af52ba5e0ef84510ca3f4c27d8f9cd55e07d90
Gerrit-Change-Number: 48463
Gerrit-PatchSet: 5
Gerrit-Owner: Giacomo Travaglini 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
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] Build failed in Jenkins: compiler-checks #148

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

See 


Changes:

[gabe.black] arch,cpu: Rename arch/generic/types.hh to pcstate.hh.

[gabe.black] arch: Promote the micropc to the base PCState class.

[gabe.black] scons: Disable the free-nonheap-object warning for gcc.

[gabe.black] cpu: Use the newly promoted uReset in the minor CPU.

[gabe.black] cpu: Add a mechanism which lets a reg class name its members.

[Giacomo Travaglini] base: Add an exclude method to the AddrRange class

[Giacomo Travaglini] python: Expose the AddrRange exclude to the python world

[gabe.black] arm: Make the misc reg class return the name of misc regs.

[gabe.black] cpu-minor: Use the RegClassInfo::regName method instead of THE_ISA.

[gabe.black] misc: Replace THE_ISA macro with IS_NULL_ISA.


--
Started by timer
Running as SYSTEM
Building in workspace 
Selected Git installation does not exist. Using Default
The recommended git tool is: NONE
No credentials specified
 > git rev-parse --resolve-git-dir 
 >  # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url https://gem5.googlesource.com/public/gem5 # 
 > timeout=10
Fetching upstream changes from https://gem5.googlesource.com/public/gem5
 > git --version # timeout=10
 > git --version # 'git version 2.25.1'
 > git fetch --tags --force --progress -- 
 > https://gem5.googlesource.com/public/gem5 
 > +refs/heads/*:refs/remotes/origin/* # timeout=10
 > git rev-parse refs/remotes/origin/develop^{commit} # timeout=10
Checking out Revision 91e24ba776c89d15c693981e7498a6c047331fef 
(refs/remotes/origin/develop)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f 91e24ba776c89d15c693981e7498a6c047331fef # timeout=10
Commit message: "misc: Replace THE_ISA macro with IS_NULL_ISA."
 > git rev-list --no-walk 1bf0b844ffc6416d8dc50c84dc22efefb97cccf4 # timeout=10
[compiler-checks] $ /bin/sh -xe /tmp/jenkins8150463551873051118.sh
+ ./tests/compiler-tests.sh -j 12
Starting build tests with 'gcc-version-10'...
'gcc-version-10' was found in the comprehensive tests. All ISAs will be built.
  * Building target 'ARM_MESI_Three_Level.opt' with 'gcc-version-10'...
Done.
  * Building target 'ARM_MESI_Three_Level.fast' with 'gcc-version-10'...
Done.
  * Building target 'GCN3_X86.opt' with 'gcc-version-10'...
Done.
  * Building target 'GCN3_X86.fast' with 'gcc-version-10'...
Done.
  * Building target 'NULL.opt' with 'gcc-version-10'...
Done.
  * Building target 'NULL.fast' with 'gcc-version-10'...
Done.
  * Building target 'NULL_MOESI_CMP_token.opt' with 'gcc-version-10'...
Done.
  * Building target 'NULL_MOESI_CMP_token.fast' with 'gcc-version-10'...
Done.
  * Building target 'NULL_MOESI_CMP_directory.opt' with 'gcc-version-10'...
Done.
  * Building target 'NULL_MOESI_CMP_directory.fast' with 'gcc-version-10'...
Done.
  * Building target 'Garnet_standalone.opt' with 'gcc-version-10'...
Done.
  * Building target 'Garnet_standalone.fast' with 'gcc-version-10'...
Done.
  * Building target 'NULL_MOESI_hammer.opt' with 'gcc-version-10'...
Done.
  * Building target 'NULL_MOESI_hammer.fast' with 'gcc-version-10'...
Done.
  * Building target 'ARM_MOESI_hammer.opt' with 'gcc-version-10'...
Done.
  * Building target 'ARM_MOESI_hammer.fast' with 'gcc-version-10'...
Done.
  * Building target 'RISCV.opt' with 'gcc-version-10'...
Done.
  * Building target 'RISCV.fast' with 'gcc-version-10'...
Done.
  * Building target 'X86_MESI_Two_Level.opt' with 'gcc-version-10'...
Done.
  * Building target 'X86_MESI_Two_Level.fast' with 'gcc-version-10'...
Done.
  * Building target 'NULL_MESI_Two_Level.opt' with 'gcc-version-10'...
Done.
  * Building target 'NULL_MESI_Two_Level.fast' with 'gcc-version-10'...
Done.
  * Building target 'MIPS.opt' with 'gcc-version-10'...
Done.
  * Building target 'MIPS.fast' with 'gcc-version-10'...
Done.
  * Building target 'X86.opt' with 'gcc-version-10'...
Done.
  * Building target 'X86.fast' with 'gcc-version-10'...
Done.
  * Building target 'ARM.opt' with 'gcc-version-10'...
Done.
  * Building target 'ARM.fast' with 'gcc-version-10'...
Done.
  * Building target 'X86_MOESI_AMD_Base.opt' with 'gcc-version-10'...
Done.
  * Building target 'X86_MOESI_AMD_Base.fast' with 'gcc-version-10'...
Done.
  * Building target 'POWER.opt' with 'gcc-version-10'...
Done.
  * Building target 'POWER.fast' with 'gcc-version-10'...
Done.
  * Building target 'ARM_MESI_Three_Level_HTM.opt' with 'gcc-version-10'...
Done.
  * Building target 'ARM_MESI_Three_Level_HTM.fast' with 'gcc-version-10'...
Done.
  * Building target 'SPARC.opt' with 'gcc-version-10'...
Done.
  * Building target 'SPARC.fast' with 'gcc-version-10'...
Done.
Starting build tests with