[gem5-dev] Change in gem5/gem5[develop]: sim: Eliminate the generic PseudoInstABI.

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


Change subject: sim: Eliminate the generic PseudoInstABI.
..

sim: Eliminate the generic PseudoInstABI.

Calls to gem5 ops are now handled by locally defined ABIs in each of the
ISAs that support them.

Change-Id: I30aac7b49fa8dc8e18aa7724338d1fd2adacda90
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39319
Reviewed-by: Jason Lowe-Power 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/sim/pseudo_inst.hh
1 file changed, 3 insertions(+), 24 deletions(-)

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



diff --git a/src/sim/pseudo_inst.hh b/src/sim/pseudo_inst.hh
index d244adb..b0b65c6 100644
--- a/src/sim/pseudo_inst.hh
+++ b/src/sim/pseudo_inst.hh
@@ -45,35 +45,14 @@

 class ThreadContext;

-#include "arch/utility.hh"
 #include "base/bitfield.hh"
+#include "base/logging.hh"
+#include "base/trace.hh"
 #include "base/types.hh" // For Tick and Addr data types.
+#include "cpu/thread_context.hh"
 #include "debug/PseudoInst.hh"
 #include "sim/guest_abi.hh"

-struct PseudoInstABI
-{
-using State = int;
-};
-
-namespace GuestABI
-{
-
-template <>
-struct Argument
-{
-static uint64_t
-get(ThreadContext *tc, PseudoInstABI::State )
-{
-uint64_t result =
-TheISA::getArgument(tc, state, sizeof(uint64_t), false);
-state++;
-return result;
-}
-};
-
-} // namespace GuestABI
-
 namespace PseudoInst
 {


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39319
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: I30aac7b49fa8dc8e18aa7724338d1fd2adacda90
Gerrit-Change-Number: 39319
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: ext: Replace Queue.Empty with queue.empty

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


Change subject: ext: Replace Queue.Empty with queue.empty
..

ext: Replace Queue.Empty with queue.empty

Queue.Empty is not an exception in python3
(Queue has been renamed to queue)

Change-Id: I82555d96608094fa47990f888fd11663379547bc
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40135
Maintainer: Bobby R. Bruce 
Reviewed-by: Jason Lowe-Power 
Tested-by: kokoro 
---
M ext/testlib/handlers.py
1 file changed, 3 insertions(+), 3 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Bobby R. Bruce: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/ext/testlib/handlers.py b/ext/testlib/handlers.py
index b62322f..fa7aea9 100644
--- a/ext/testlib/handlers.py
+++ b/ext/testlib/handlers.py
@@ -44,7 +44,7 @@
 import testlib.state as state
 import testlib.terminal as terminal

-from queue import Queue
+from queue import Queue, Empty
 from testlib.configuration import constants


@@ -383,7 +383,7 @@
 raise
 except EOFError:
 return
-except Queue.Empty:
+except Empty:
 continue

 def _drain(self):
@@ -395,7 +395,7 @@
 raise
 except EOFError:
 return
-except Queue.Empty:
+except Empty:
 return

 def _handle(self, record):

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40135
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: I82555d96608094fa47990f888fd11663379547bc
Gerrit-Change-Number: 40135
Gerrit-PatchSet: 2
Gerrit-Owner: Giacomo Travaglini 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: ext: Fix Queue import in handlers.py

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



Change subject: ext: Fix Queue import in handlers.py
..

ext: Fix Queue import in handlers.py

Previously this caused the following error when trying to run tests:

```
Traceback (most recent call last):
  File "/fasthome/bbruce/gem5/tests/../ext/testlib/handlers.py", line 392,  
in _drain

item = self.queue.get(block=False)
  File "/usr/lib/python3.6/multiprocessing/queues.py", line 107, in get
raise Empty
queue.Empty
```

This was due to a change in the importing of the queue package in
https://gem5-review.googlesource.com/c/public/gem5/+/39759

Change-Id: I17dacba0948b48cba91cca472f028bf10b277ea2
---
M ext/testlib/handlers.py
1 file changed, 1 insertion(+), 1 deletion(-)



diff --git a/ext/testlib/handlers.py b/ext/testlib/handlers.py
index b62322f..c96cc8b 100644
--- a/ext/testlib/handlers.py
+++ b/ext/testlib/handlers.py
@@ -44,7 +44,7 @@
 import testlib.state as state
 import testlib.terminal as terminal

-from queue import Queue
+import queue as Queue
 from testlib.configuration import constants



--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40155
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: I17dacba0948b48cba91cca472f028bf10b277ea2
Gerrit-Change-Number: 40155
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]: configs: Use MmioVirtIO for disk image in baremetal.py

2021-01-29 Thread Giacomo Travaglini (Gerrit) via gem5-dev
Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/39995 )


Change subject: configs: Use MmioVirtIO for disk image in baremetal.py
..

configs: Use MmioVirtIO for disk image in baremetal.py

The baremetal platform is the platform we use for running
user supplied binaries on baremetal hardware.
(simply put, it runs provided binaries without adding
a gem5 bootloader)

Some layers of this software stack might not have a pci driver.
This might be the case for firmware images like edkII
which needs to use a block device to extract the bootloader
and/or the kernel image. Those can use the memory mapped
(in host domain) virtio block device which is already
part of the VExpress_GEM5 platforms

Change-Id: I9c6ba7e1b4566a3999fd9ba20a2bebe191dc3ef8
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39995
Reviewed-by: Andreas Sandberg 
Reviewed-by: Richard Cooper 
Maintainer: Andreas Sandberg 
Tested-by: kokoro 
---
M configs/example/arm/baremetal.py
1 file changed, 3 insertions(+), 14 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  Richard Cooper: Looks good to me, but someone else must approve
  kokoro: Regressions pass



diff --git a/configs/example/arm/baremetal.py  
b/configs/example/arm/baremetal.py

index 011883b..29e9b48 100644
--- a/configs/example/arm/baremetal.py
+++ b/configs/example/arm/baremetal.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2016-2017,2019-2020 ARM Limited
+# Copyright (c) 2016-2017,2019-2021 ARM Limited
 # All rights reserved.
 #
 # The license below extends only to copyright in the software and shall
@@ -113,24 +113,13 @@
 cmd_line = " ".join([ object_file ] + args.args)
 )

-# Add the PCI devices we need for this system. The base system
-# doesn't have any PCI devices by default since they are assumed
-# to be added by the configurastion scripts needin them.
-pci_devices = []
 if args.disk_image:
 # Create a VirtIO block device for the system's boot
 # disk. Attach the disk image using gem5's Copy-on-Write
 # functionality to avoid writing changes to the stored copy of
 # the disk image.
-system.disk = PciVirtIO(vio=VirtIOBlock(
-image=create_cow_image(args.disk_image)))
-pci_devices.append(system.disk)
-
-# Attach the PCI devices to the system. The helper method in the
-# system assigns a unique PCI bus ID to each of the devices and
-# connects them to the IO bus.
-for dev in pci_devices:
-system.attach_pci(dev)
+system.realview.vio[0].vio = VirtIOBlock(
+image=create_cow_image(args.disk_image))

 # Wire up the system's memory system
 system.connect()

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39995
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: I9c6ba7e1b4566a3999fd9ba20a2bebe191dc3ef8
Gerrit-Change-Number: 39995
Gerrit-PatchSet: 3
Gerrit-Owner: Giacomo Travaglini 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Nikos Nikoleris 
Gerrit-Reviewer: Richard Cooper 
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]: ext: testlib loading tests from multiple directories

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



Change subject: ext: testlib loading tests from multiple directories
..

ext: testlib loading tests from multiple directories

We currently run regressions with the following command line

./main.py run [...] 

Where  is the positional argument pointing to the tests root
directory: Testlib will walk through the directory and load every
testsuite it encounters in its path.

./main.py run [...]   ...

Allowing testlib to load tests from multiple directories will make it
possible to load out of tree regressions (living in an EXTRAS repository
for example)

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

Change-Id: I802d8753a18f4dfb00347252f031b5438e9be672
Signed-off-by: Giacomo Travaglini 
---
M TESTING.md
M ext/testlib/configuration.py
M ext/testlib/main.py
3 files changed, 27 insertions(+), 9 deletions(-)



diff --git a/TESTING.md b/TESTING.md
index 17aeff9..88d1f29 100644
--- a/TESTING.md
+++ b/TESTING.md
@@ -63,6 +63,20 @@
 The above is the *minumum* you should run before posting a patch to
 https://gem5-review.googlesource.com

+## Running tests from multiple directories
+
+The command line above will walk the directory tree starting from the cwd
+(tests), and it will run every test it encounters in its path. It is  
possible

+to specify multiple root directories by providing several positional
+arguments:
+
+```shell
+./main.py run   [...]
+```
+
+This will load every test in directory1 and directory2 (and their
+subdirectories).
+
 ## Specifying a subset of tests to run

 You can use the tag query interface to specify the exact tests you want to  
run.

diff --git a/ext/testlib/configuration.py b/ext/testlib/configuration.py
index f2d93d6..1fffab4 100644
--- a/ext/testlib/configuration.py
+++ b/ext/testlib/configuration.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2020 ARM Limited
+# Copyright (c) 2020-2021 ARM Limited
 # All rights reserved
 #
 # The license below extends only to copyright in the software and shall
@@ -493,10 +493,11 @@
 # A list of common arguments/flags used across cli parsers.
 common_args = [
 Argument(
-'directory',
-nargs='?',
-default=os.getcwd(),
-help='Directory to start searching for tests in'),
+'directories',
+nargs='*',
+default=[os.getcwd()],
+help='Space separated list of directories to start searching '
+ 'for tests in'),
 Argument(
 '--exclude-tags',
 action=StorePositionalTagsAction,
@@ -646,7 +647,7 @@

 common_args.uid.add_to(parser)
 common_args.skip_build.add_to(parser)
-common_args.directory.add_to(parser)
+common_args.directories.add_to(parser)
 common_args.build_dir.add_to(parser)
 common_args.base_dir.add_to(parser)
 common_args.bin_path.add_to(parser)
@@ -703,7 +704,7 @@
 help='Quiet output (machine readable).'
 ).add_to(parser)

-common_args.directory.add_to(parser)
+common_args.directories.add_to(parser)
 common_args.bin_path.add_to(parser)
 common_args.isa.add_to(parser)
 common_args.variant.add_to(parser)
@@ -722,7 +723,7 @@
 super(RerunParser, self).__init__(parser)

 common_args.skip_build.add_to(parser)
-common_args.directory.add_to(parser)
+common_args.directories.add_to(parser)
 common_args.build_dir.add_to(parser)
 common_args.base_dir.add_to(parser)
 common_args.bin_path.add_to(parser)
diff --git a/ext/testlib/main.py b/ext/testlib/main.py
index d2ae5a9..6087a8e 100644
--- a/ext/testlib/main.py
+++ b/ext/testlib/main.py
@@ -205,7 +205,10 @@
 testloader = loader_mod.Loader()
 log.test_log.message(terminal.separator())
 log.test_log.message('Loading Tests', bold=True)
-testloader.load_root(configuration.config.directory)
+
+for root in configuration.config.directories:
+testloader.load_root(root)
+
 return testloader

 def do_list():

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40136
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: I802d8753a18f4dfb00347252f031b5438e9be672
Gerrit-Change-Number: 40136
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: ext: Replace Queue.Empty with queue.empty

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



Change subject: ext: Replace Queue.Empty with queue.empty
..

ext: Replace Queue.Empty with queue.empty

Queue.Empty is not an exception in python3
(Queue has been renamed to queue)

Change-Id: I82555d96608094fa47990f888fd11663379547bc
Signed-off-by: Giacomo Travaglini 
---
M ext/testlib/handlers.py
1 file changed, 3 insertions(+), 3 deletions(-)



diff --git a/ext/testlib/handlers.py b/ext/testlib/handlers.py
index b62322f..fa7aea9 100644
--- a/ext/testlib/handlers.py
+++ b/ext/testlib/handlers.py
@@ -44,7 +44,7 @@
 import testlib.state as state
 import testlib.terminal as terminal

-from queue import Queue
+from queue import Queue, Empty
 from testlib.configuration import constants


@@ -383,7 +383,7 @@
 raise
 except EOFError:
 return
-except Queue.Empty:
+except Empty:
 continue

 def _drain(self):
@@ -395,7 +395,7 @@
 raise
 except EOFError:
 return
-except Queue.Empty:
+except Empty:
 return

 def _handle(self, record):

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40135
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: I82555d96608094fa47990f888fd11663379547bc
Gerrit-Change-Number: 40135
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Fix style in decoder.hh.

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


Change subject: arch-arm: Fix style in decoder.hh.
..

arch-arm: Fix style in decoder.hh.

Change-Id: I45cf1fefc6145393abec2de12e74816c0c8ac0e9
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40096
Reviewed-by: Daniel Carvalho 
Reviewed-by: Giacomo Travaglini 
Maintainer: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/arch/arm/decoder.hh
1 file changed, 6 insertions(+), 3 deletions(-)

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



diff --git a/src/arch/arm/decoder.hh b/src/arch/arm/decoder.hh
index 598e865..fa5b15e 100644
--- a/src/arch/arm/decoder.hh
+++ b/src/arch/arm/decoder.hh
@@ -169,7 +169,8 @@
  * @param mach_inst A pre-decoded instruction
  * @retval A pointer to the corresponding StaticInst object.
  */
-StaticInstPtr decode(ExtMachInst mach_inst, Addr addr)
+StaticInstPtr
+decode(ExtMachInst mach_inst, Addr addr)
 {
 return defaultCache.decode(this, mach_inst, addr);
 }
@@ -197,13 +198,15 @@


   public: // ARM-specific decoder state manipulation
-void setContext(FPSCR fpscr)
+void
+setContext(FPSCR fpscr)
 {
 fpscrLen = fpscr.len;
 fpscrStride = fpscr.stride;
 }

-void setSveLen(uint8_t len)
+void
+setSveLen(uint8_t len)
 {
 sveLen = len;
 }

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40096
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: I45cf1fefc6145393abec2de12e74816c0c8ac0e9
Gerrit-Change-Number: 40096
Gerrit-PatchSet: 2
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: base: Style fixes in base/refcnt.hh

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


Change subject: base: Style fixes in base/refcnt.hh
..

base: Style fixes in base/refcnt.hh

Change-Id: I8f4b2710bea1fe15baa1b482ff62fbab645a3690
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40095
Reviewed-by: Daniel Carvalho 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/base/refcnt.hh
1 file changed, 43 insertions(+), 16 deletions(-)

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



diff --git a/src/base/refcnt.hh b/src/base/refcnt.hh
index 263666e..3106a6f 100644
--- a/src/base/refcnt.hh
+++ b/src/base/refcnt.hh
@@ -96,7 +96,12 @@

 /// Decrement the reference count and destroy the object if all
 /// references are gone.
-void decref() const { if (--count <= 0) delete this; }
+void
+decref() const
+{
+if (--count <= 0)
+delete this;
+}
 };

 /**
@@ -228,11 +233,15 @@
 const RefCountingPtr =(T *p) { set(p); return *this; }

 /// Copy the pointer from another RefCountingPtr
-const RefCountingPtr =(const RefCountingPtr )
-{ return operator=(r.data); }
+const RefCountingPtr &
+operator=(const RefCountingPtr )
+{
+return operator=(r.data);
+}

 /// Move-assign the pointer from another RefCountingPtr
-const RefCountingPtr =(RefCountingPtr&& r)
+const RefCountingPtr &
+operator=(RefCountingPtr&& r)
 {
 /* This happens regardless of whether the pointer is the same or  
not,
  * because of the move semantics, the rvalue needs to  
be 'destroyed'.

@@ -252,36 +261,54 @@

 /// Check for equality of two reference counting pointers.
 template
-inline bool operator==(const RefCountingPtr , const RefCountingPtr  
)

-{ return l.get() == r.get(); }
+inline bool
+operator==(const RefCountingPtr , const RefCountingPtr )
+{
+return l.get() == r.get();
+}

 /// Check for equality of of a reference counting pointers and a
 /// regular pointer
 template
-inline bool operator==(const RefCountingPtr , const T *r)
-{ return l.get() == r; }
+inline bool
+operator==(const RefCountingPtr , const T *r)
+{
+return l.get() == r;
+}

 /// Check for equality of of a reference counting pointers and a
 /// regular pointer
 template
-inline bool operator==(const T *l, const RefCountingPtr )
-{ return l == r.get(); }
+inline bool
+operator==(const T *l, const RefCountingPtr )
+{
+return l == r.get();
+}

 /// Check for inequality of two reference counting pointers.
 template
-inline bool operator!=(const RefCountingPtr , const RefCountingPtr  
)

-{ return l.get() != r.get(); }
+inline bool
+operator!=(const RefCountingPtr , const RefCountingPtr )
+{
+return l.get() != r.get();
+}

 /// Check for inequality of of a reference counting pointers and a
 /// regular pointer
 template
-inline bool operator!=(const RefCountingPtr , const T *r)
-{ return l.get() != r; }
+inline bool
+operator!=(const RefCountingPtr , const T *r)
+{
+return l.get() != r;
+}

 /// Check for inequality of of a reference counting pointers and a
 /// regular pointer
 template
-inline bool operator!=(const T *l, const RefCountingPtr )
-{ return l != r.get(); }
+inline bool
+operator!=(const T *l, const RefCountingPtr )
+{
+return l != r.get();
+}

 #endif // __BASE_REFCNT_HH__

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40095
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: I8f4b2710bea1fe15baa1b482ff62fbab645a3690
Gerrit-Change-Number: 40095
Gerrit-PatchSet: 2
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch-riscv: fix unintentionally CSR bit overwritten in different mode

2021-01-29 Thread Cui Jin (Gerrit) via gem5-dev
Cui Jin has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/39036 )


Change subject: arch-riscv: fix unintentionally CSR bit overwritten in  
different mode

..

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

Some CSR register is physically shared between different privilige
level. Current implementation of CSR setting only considers to verify
the bits visable in current privilige level, and directly writes the
masked bits back to register. This leads to other bits invisable
to current mode is overwritten and wrong behavior across the modes.
Thus, CSR updating should always keep the bits value for other modes.
e.g. disabling interrupt in S mode with setting
SSTATUS SIE bit will lead to clear MIE bit as well (the interrupt
is disabled unintentionally).

All CSR register sharing same physical register in different mode
may have similar issue. I only fixed some important ones.

The fix is verified in FS.

Jira Issue: https://gem5.atlassian.net/browse/GEM5-860

Change-Id: I34d4766a4b483b5add2c3bbefd28b21b9abf37f6
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39036
Tested-by: kokoro 
Reviewed-by: Ayaz Akram 
Maintainer: Jason Lowe-Power 
---
M src/arch/riscv/isa/formats/standard.isa
1 file changed, 19 insertions(+), 9 deletions(-)

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



diff --git a/src/arch/riscv/isa/formats/standard.isa  
b/src/arch/riscv/isa/formats/standard.isa

index a32326f..4ed35c9 100644
--- a/src/arch/riscv/isa/formats/standard.isa
+++ b/src/arch/riscv/isa/formats/standard.isa
@@ -356,6 +356,7 @@
 break;
 }
 auto mask = CSRMasks.find(csr);
+auto olddata_all = olddata;
 if (mask != CSRMasks.end())
 olddata &= mask->second;
 DPRINTF(RiscvMisc, "Reading CSR %s: %#x\n", CSRData.at(csr).name,
@@ -374,23 +375,32 @@
 fault = std::make_shared(
 error, machInst);
 } else {
-DPRINTF(RiscvMisc, "Writing %#x to CSR %s.\n",  
data,

+auto newdata_all = data;
+if (mask != CSRMasks.end()) {
+// we must keep those original bits not in mask
+// olddata and data only contain thebits  
visable

+// in current privilige level
+newdata_all = (olddata_all & (~mask->second))
+  | data;
+}
+DPRINTF(RiscvMisc, "Writing %#x to CSR %s.\n",
+newdata_all,
 CSRData.at(csr).name);
-INTERRUPT oldinterrupt = olddata;
-INTERRUPT newinterrupt = data;
 switch (csr) {
   case CSR_FCSR:
 xc->setMiscReg(MISCREG_FFLAGS, bits(data, 4,  
0));

 xc->setMiscReg(MISCREG_FRM, bits(data, 7, 5));
 break;
   case CSR_MIP: case CSR_MIE:
-if (oldinterrupt.mei != newinterrupt.mei ||
-oldinterrupt.mti != newinterrupt.mti ||
-oldinterrupt.msi != newinterrupt.msi) {
- 
xc->setMiscReg(CSRData.at(csr).physIndex,data);

+  case CSR_SIP: case CSR_SIE:
+  case CSR_UIP: case CSR_UIE:
+  case CSR_MSTATUS: case CSR_SSTATUS: case  
CSR_USTATUS:

+if (newdata_all != olddata_all) {
+xc->setMiscReg(CSRData.at(csr).physIndex,
+   newdata_all);
 } else {
-std::string error = "Interrupt m bits are "
-"read-only\n";
+std::string error = "Only bits in mask  
are "

+"allowed to be set\n";
 fault = std::make_shared(
 error, machInst);
 }

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39036
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: I34d4766a4b483b5add2c3bbefd28b21b9abf37f6
Gerrit-Change-Number: 39036
Gerrit-PatchSet: 4
Gerrit-Owner: Cui Jin 
Gerrit-Reviewer: Ayaz Akram 
Gerrit-Reviewer: Cui Jin 
Gerrit-Reviewer: Jason Lowe-Power 

[gem5-dev] Build failed in Jenkins: Weekly #1

2021-01-29 Thread jenkins-no-reply--- via gem5-dev
See 

Changes:


--
[...truncated 5.03 MB...]
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is deprecated. `slave` is now called `in_ports`
warn: coalescer.slave is 

[gem5-dev] Change in gem5/gem5[develop]: arch-riscv: Fixing interrupt handling order and effect of mideleg

2021-01-29 Thread Peter Yuen (Gerrit) via gem5-dev
Peter Yuen has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40076 )



Change subject: arch-riscv: Fixing interrupt handling order and effect of  
mideleg

..

arch-riscv: Fixing interrupt handling order and effect of mideleg

This patch fixes the issues listed in:
https://gem5.atlassian.net/browse/GEM5-887
https://gem5.atlassian.net/browse/GEM5-889

The code change has been verified by booting FS linux.
Software, timer and external interrupts work as expected.

Change-Id: I6ce4fc843d2e0338355152c3fc33c966d6b4a481
---
M src/arch/riscv/interrupts.hh
1 file changed, 19 insertions(+), 6 deletions(-)



diff --git a/src/arch/riscv/interrupts.hh b/src/arch/riscv/interrupts.hh
index e1460ab..10638dd 100644
--- a/src/arch/riscv/interrupts.hh
+++ b/src/arch/riscv/interrupts.hh
@@ -72,16 +72,24 @@
 {
 INTERRUPT mask = 0;
 STATUS status = tc->readMiscReg(MISCREG_STATUS);
+INTERRUPT mideleg = tc->readMiscReg(MISCREG_MIDELEG);
+INTERRUPT sideleg = tc->readMiscReg(MISCREG_SIDELEG);
 PrivilegeMode prv = (PrivilegeMode)tc->readMiscReg(MISCREG_PRV);
 switch (prv) {
 case PRV_U:
-mask.mei = mask.mti = mask.msi = 1;
-mask.sei = mask.sti = mask.ssi = 1;
+mask.mei = mideleg.mei ^ status.uie;
+mask.mti = mideleg.mti ^ status.uie;
+mask.msi = mideleg.msi ^ status.uie;
+mask.sei = sideleg.sei ^ status.uie;
+mask.sti = sideleg.sti ^ status.uie;
+mask.ssi = sideleg.ssi ^ status.uie;
 if (status.uie)
 mask.uei = mask.uti = mask.usi = 1;
 break;
 case PRV_S:
-mask.mei = mask.mti = mask.msi = 1;
+mask.mei = mideleg.mei ^ status.sie;
+mask.mti = mideleg.mti ^ status.sie;
+mask.msi = mideleg.msi ^ status.sie;
 if (status.sie)
 mask.sei = mask.sti = mask.ssi = 1;
 mask.uei = mask.uti = mask.usi = 0;
@@ -111,9 +119,14 @@
 {
 assert(checkInterrupts());
 std::bitset mask = globalMask();
-for (int c = 0; c < NumInterruptTypes; c++)
-if (checkInterrupt(c) && mask[c])
-return std::make_shared(c);
+const std::vector interrupt_order {
+INT_EXT_MACHINE, INT_TIMER_MACHINE, INT_SOFTWARE_MACHINE,
+INT_EXT_SUPER, INT_TIMER_SUPER, INT_SOFTWARE_SUPER,
+INT_EXT_USER, INT_TIMER_USER, INT_SOFTWARE_USER
+};
+for (const int  : interrupt_order)
+if (checkInterrupt(id) && mask[id])
+return std::make_shared(id);
 return NoFault;
 }


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40076
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: I6ce4fc843d2e0338355152c3fc33c966d6b4a481
Gerrit-Change-Number: 40076
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Yuen 
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: Get rid of the RenameMode template class.

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



Change subject: arch,cpu: Get rid of the RenameMode template class.
..

arch,cpu: Get rid of the RenameMode template class.

There is no way to make this sort of template work with more than one
ISA at a time, and it's also more complex than it needs to be,
particularly since the methods within it are never used in performance
critical code. Using virtual functions is also simpler and uses less
code.

Change-Id: I0baa1a651fa656420f6f90776572f8700a6d7cab
---
M src/arch/arm/isa.cc
M src/arch/arm/isa.hh
M src/arch/arm/utility.cc
M src/arch/generic/isa.hh
D src/arch/generic/traits.hh
M src/cpu/o3/cpu.cc
M src/cpu/o3/thread_context_impl.hh
7 files changed, 29 insertions(+), 113 deletions(-)



diff --git a/src/arch/arm/isa.cc b/src/arch/arm/isa.cc
index f4fabc1..c9ddce0 100644
--- a/src/arch/arm/isa.cc
+++ b/src/arch/arm/isa.cc
@@ -61,8 +61,7 @@
 {

 ISA::ISA(const Params ) : BaseISA(p), system(NULL),
-_decoderFlavor(p.decoderFlavor), _vecRegRenameMode(Enums::Full),
-pmu(p.pmu), impdefAsNop(p.impdef_nop),
+_decoderFlavor(p.decoderFlavor), pmu(p.pmu), impdefAsNop(p.impdef_nop),
 afterStartup(false)
 {
 miscRegs[MISCREG_SCTLR_RST] = 0;
@@ -107,10 +106,6 @@
 haveTME = true;
 }

-// Initial rename mode depends on highestEL
-const_cast(_vecRegRenameMode) =
-highestELIs64 ? Enums::Full : Enums::Elem;
-
 selfDebug = new SelfDebug();
 initializeMiscRegMetadata();
 preUnflattenMiscReg();
diff --git a/src/arch/arm/isa.hh b/src/arch/arm/isa.hh
index dce5e37..1389570 100644
--- a/src/arch/arm/isa.hh
+++ b/src/arch/arm/isa.hh
@@ -49,7 +49,6 @@
 #include "arch/arm/tlb.hh"
 #include "arch/arm/types.hh"
 #include "arch/generic/isa.hh"
-#include "arch/generic/traits.hh"
 #include "debug/Checkpoint.hh"
 #include "enums/DecoderFlavor.hh"
 #include "enums/VecRegRenameMode.hh"
@@ -70,7 +69,6 @@

 // Micro Architecture
 const Enums::DecoderFlavor _decoderFlavor;
-const Enums::VecRegRenameMode _vecRegRenameMode;

 /** Dummy device for to handle non-existing ISA devices */
 DummyISADevice dummyDevice;
@@ -876,9 +874,15 @@
 }

 Enums::VecRegRenameMode
-vecRegRenameMode() const
+initVecRegRenameMode() const override
 {
-return _vecRegRenameMode;
+return highestELIs64 ? Enums::Full : Enums::Elem;
+}
+
+Enums::VecRegRenameMode
+vecRegRenameMode(ThreadContext *_tc) const override
+{
+return _tc->pcState().aarch64() ? Enums::Full : Enums::Elem;
 }

 typedef ArmISAParams Params;
@@ -889,32 +893,4 @@
 };
 }

-template<>
-struct RenameMode
-{
-static Enums::VecRegRenameMode
-init(const BaseISA* isa)
-{
-auto arm_isa = dynamic_cast(isa);
-assert(arm_isa);
-return arm_isa->vecRegRenameMode();
-}
-
-static Enums::VecRegRenameMode
-mode(const ArmISA::PCState& pc)
-{
-if (pc.aarch64()) {
-return Enums::Full;
-} else {
-return Enums::Elem;
-}
-}
-
-static bool
-equalsInit(const BaseISA* isa1, const BaseISA* isa2)
-{
-return init(isa1) == init(isa2);
-}
-};
-
 #endif
diff --git a/src/arch/arm/utility.cc b/src/arch/arm/utility.cc
index 93e0a78..b383214 100644
--- a/src/arch/arm/utility.cc
+++ b/src/arch/arm/utility.cc
@@ -114,7 +114,7 @@
 static void
 copyVecRegs(ThreadContext *src, ThreadContext *dest)
 {
-auto src_mode = RenameMode::mode(src->pcState());
+auto src_mode = src->getIsaPtr()->vecRegRenameMode(src);

 // The way vector registers are copied (VecReg vs VecElem) is relevant
 // in the O3 model only.
diff --git a/src/arch/generic/isa.hh b/src/arch/generic/isa.hh
index c1b8734..611281e 100644
--- a/src/arch/generic/isa.hh
+++ b/src/arch/generic/isa.hh
@@ -40,6 +40,7 @@
 #ifndef __ARCH_GENERIC_ISA_HH__
 #define __ARCH_GENERIC_ISA_HH__

+#include "enums/VecRegRenameMode.hh"
 #include "sim/sim_object.hh"

 class ThreadContext;
@@ -54,6 +55,19 @@
   public:
 virtual void takeOverFrom(ThreadContext *new_tc, ThreadContext  
*old_tc) {}

 virtual void setThreadContext(ThreadContext *_tc) { tc = _tc; }
+
+virtual Enums::VecRegRenameMode
+initVecRegRenameMode() const
+{
+return Enums::Full;
+}
+
+virtual Enums::VecRegRenameMode
+vecRegRenameMode(ThreadContext *_tc) const
+{
+return initVecRegRenameMode();
+}
+
 };

 #endif // __ARCH_GENERIC_ISA_HH__
diff --git a/src/arch/generic/traits.hh b/src/arch/generic/traits.hh
deleted file mode 100644
index 5363419..000
--- a/src/arch/generic/traits.hh
+++ /dev/null
@@ -1,68 +0,0 @@
-/*
- * Copyright (c) 2016, 2020 ARM Limited
- * All rights reserved
- *
- * The license below extends only to copyright in 

[gem5-dev] Change in gem5/gem5[develop]: arch: Correct style in the ISA base class.

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



Change subject: arch: Correct style in the ISA base class.
..

arch: Correct style in the ISA base class.

Change-Id: I1732f519bf3eab1dff8b9a9a30fc8e5e132d067d
---
M src/arch/generic/isa.hh
1 file changed, 1 insertion(+), 4 deletions(-)



diff --git a/src/arch/generic/isa.hh b/src/arch/generic/isa.hh
index df07763..c1b8734 100644
--- a/src/arch/generic/isa.hh
+++ b/src/arch/generic/isa.hh
@@ -52,10 +52,7 @@
 ThreadContext *tc = nullptr;

   public:
-virtual void
-takeOverFrom(ThreadContext *new_tc, ThreadContext *old_tc)
-{}
-
+virtual void takeOverFrom(ThreadContext *new_tc, ThreadContext  
*old_tc) {}

 virtual void setThreadContext(ThreadContext *_tc) { tc = _tc; }
 };


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40105
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: I1732f519bf3eab1dff8b9a9a30fc8e5e132d067d
Gerrit-Change-Number: 40105
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Fix style in arch/x86/types.hh.

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



Change subject: arch-x86: Fix style in arch/x86/types.hh.
..

arch-x86: Fix style in arch/x86/types.hh.

Change-Id: I5e32eea9b843d4f68adf5430516d0636317c8a57
---
M src/arch/x86/types.hh
1 file changed, 298 insertions(+), 295 deletions(-)



diff --git a/src/arch/x86/types.hh b/src/arch/x86/types.hh
index 890a9e5..9b0b034 100644
--- a/src/arch/x86/types.hh
+++ b/src/arch/x86/types.hh
@@ -48,336 +48,339 @@

 namespace X86ISA
 {
-//This really determines how many bytes are passed to the decoder.
-typedef uint64_t MachInst;

-enum Prefixes {
-NoOverride,
-ESOverride,
-CSOverride,
-SSOverride,
-DSOverride,
-FSOverride,
-GSOverride,
-RexPrefix,
-OperandSizeOverride,
-AddressSizeOverride,
-Lock,
-Rep,
-Repne,
-Vex2Prefix,
-Vex3Prefix,
-XopPrefix,
-};
+//This really determines how many bytes are passed to the decoder.
+typedef uint64_t MachInst;

-BitUnion8(LegacyPrefixVector)
-Bitfield<7, 4> decodeVal;
-Bitfield<7> repne;
-Bitfield<6> rep;
-Bitfield<5> lock;
-Bitfield<4> op;
-Bitfield<3> addr;
-//There can be only one segment override, so they share the
-//first 3 bits in the legacyPrefixes bitfield.
-Bitfield<2,0> seg;
-EndBitUnion(LegacyPrefixVector)
+enum Prefixes {
+NoOverride,
+ESOverride,
+CSOverride,
+SSOverride,
+DSOverride,
+FSOverride,
+GSOverride,
+RexPrefix,
+OperandSizeOverride,
+AddressSizeOverride,
+Lock,
+Rep,
+Repne,
+Vex2Prefix,
+Vex3Prefix,
+XopPrefix,
+};

-BitUnion8(ModRM)
-Bitfield<7,6> mod;
-Bitfield<5,3> reg;
-Bitfield<2,0> rm;
-EndBitUnion(ModRM)
+BitUnion8(LegacyPrefixVector)
+Bitfield<7, 4> decodeVal;
+Bitfield<7> repne;
+Bitfield<6> rep;
+Bitfield<5> lock;
+Bitfield<4> op;
+Bitfield<3> addr;
+//There can be only one segment override, so they share the
+//first 3 bits in the legacyPrefixes bitfield.
+Bitfield<2,0> seg;
+EndBitUnion(LegacyPrefixVector)

-BitUnion8(Sib)
-Bitfield<7,6> scale;
-Bitfield<5,3> index;
-Bitfield<2,0> base;
-EndBitUnion(Sib)
+BitUnion8(ModRM)
+Bitfield<7,6> mod;
+Bitfield<5,3> reg;
+Bitfield<2,0> rm;
+EndBitUnion(ModRM)

-BitUnion8(Rex)
-//This bit doesn't mean anything according to the ISA, but in
-//this implementation, it being set means an REX prefix was  
present.

-Bitfield<6> present;
-Bitfield<3> w;
-Bitfield<2> r;
-Bitfield<1> x;
-Bitfield<0> b;
-EndBitUnion(Rex)
+BitUnion8(Sib)
+Bitfield<7,6> scale;
+Bitfield<5,3> index;
+Bitfield<2,0> base;
+EndBitUnion(Sib)

-BitUnion8(Vex2Of3)
-// Inverted bits from the REX prefix.
-Bitfield<7> r;
-Bitfield<6> x;
-Bitfield<5> b;
-// Selector for what would be two or three byte opcode types.
-Bitfield<4, 0> m;
-EndBitUnion(Vex2Of3)
+BitUnion8(Rex)
+//This bit doesn't mean anything according to the ISA, but in
+//this implementation, it being set means an REX prefix was present.
+Bitfield<6> present;
+Bitfield<3> w;
+Bitfield<2> r;
+Bitfield<1> x;
+Bitfield<0> b;
+EndBitUnion(Rex)

-BitUnion8(Vex3Of3)
-// Bit from the REX prefix.
-Bitfield<7> w;
-// Inverted extra register index.
-Bitfield<6, 3>  v;
-// Vector length specifier.
-Bitfield<2> l;
-// Implied 66, F2, or F3 opcode prefix.
-Bitfield<1, 0> p;
-EndBitUnion(Vex3Of3)
+BitUnion8(Vex2Of3)
+// Inverted bits from the REX prefix.
+Bitfield<7> r;
+Bitfield<6> x;
+Bitfield<5> b;
+// Selector for what would be two or three byte opcode types.
+Bitfield<4, 0> m;
+EndBitUnion(Vex2Of3)

-BitUnion8(Vex2Of2)
-// Inverted bit from the REX prefix.
-Bitfield<7> r;
-// Inverted extra register index.
-Bitfield<6, 3>  v;
-// Vector length specifier
-Bitfield<2> l;
-// Implied 66, F2, or F3 opcode prefix.
-Bitfield<1, 0> p;
-EndBitUnion(Vex2Of2)
+BitUnion8(Vex3Of3)
+// Bit from the REX prefix.
+Bitfield<7> w;
+// Inverted extra register index.
+Bitfield<6, 3>  v;
+// Vector length specifier.
+Bitfield<2> l;
+// Implied 66, F2, or F3 opcode prefix.
+Bitfield<1, 0> p;
+EndBitUnion(Vex3Of3)

-BitUnion8(VexInfo)
-// Extra register index.
-Bitfield<6, 3> v;
-// Vector length specifier.
-Bitfield<2> l;
-// Whether the VEX prefix was used.
-Bitfield<0> present;
-EndBitUnion(VexInfo)
+BitUnion8(Vex2Of2)
+

[gem5-dev] Change in gem5/gem5[develop]: base: Style fixes in base/refcnt.hh

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



Change subject: base: Style fixes in base/refcnt.hh
..

base: Style fixes in base/refcnt.hh

Change-Id: I8f4b2710bea1fe15baa1b482ff62fbab645a3690
---
M src/base/refcnt.hh
1 file changed, 43 insertions(+), 16 deletions(-)



diff --git a/src/base/refcnt.hh b/src/base/refcnt.hh
index 263666e..3106a6f 100644
--- a/src/base/refcnt.hh
+++ b/src/base/refcnt.hh
@@ -96,7 +96,12 @@

 /// Decrement the reference count and destroy the object if all
 /// references are gone.
-void decref() const { if (--count <= 0) delete this; }
+void
+decref() const
+{
+if (--count <= 0)
+delete this;
+}
 };

 /**
@@ -228,11 +233,15 @@
 const RefCountingPtr =(T *p) { set(p); return *this; }

 /// Copy the pointer from another RefCountingPtr
-const RefCountingPtr =(const RefCountingPtr )
-{ return operator=(r.data); }
+const RefCountingPtr &
+operator=(const RefCountingPtr )
+{
+return operator=(r.data);
+}

 /// Move-assign the pointer from another RefCountingPtr
-const RefCountingPtr =(RefCountingPtr&& r)
+const RefCountingPtr &
+operator=(RefCountingPtr&& r)
 {
 /* This happens regardless of whether the pointer is the same or  
not,
  * because of the move semantics, the rvalue needs to  
be 'destroyed'.

@@ -252,36 +261,54 @@

 /// Check for equality of two reference counting pointers.
 template
-inline bool operator==(const RefCountingPtr , const RefCountingPtr  
)

-{ return l.get() == r.get(); }
+inline bool
+operator==(const RefCountingPtr , const RefCountingPtr )
+{
+return l.get() == r.get();
+}

 /// Check for equality of of a reference counting pointers and a
 /// regular pointer
 template
-inline bool operator==(const RefCountingPtr , const T *r)
-{ return l.get() == r; }
+inline bool
+operator==(const RefCountingPtr , const T *r)
+{
+return l.get() == r;
+}

 /// Check for equality of of a reference counting pointers and a
 /// regular pointer
 template
-inline bool operator==(const T *l, const RefCountingPtr )
-{ return l == r.get(); }
+inline bool
+operator==(const T *l, const RefCountingPtr )
+{
+return l == r.get();
+}

 /// Check for inequality of two reference counting pointers.
 template
-inline bool operator!=(const RefCountingPtr , const RefCountingPtr  
)

-{ return l.get() != r.get(); }
+inline bool
+operator!=(const RefCountingPtr , const RefCountingPtr )
+{
+return l.get() != r.get();
+}

 /// Check for inequality of of a reference counting pointers and a
 /// regular pointer
 template
-inline bool operator!=(const RefCountingPtr , const T *r)
-{ return l.get() != r; }
+inline bool
+operator!=(const RefCountingPtr , const T *r)
+{
+return l.get() != r;
+}

 /// Check for inequality of of a reference counting pointers and a
 /// regular pointer
 template
-inline bool operator!=(const T *l, const RefCountingPtr )
-{ return l != r.get(); }
+inline bool
+operator!=(const T *l, const RefCountingPtr )
+{
+return l != r.get();
+}

 #endif // __BASE_REFCNT_HH__

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40095
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: I8f4b2710bea1fe15baa1b482ff62fbab645a3690
Gerrit-Change-Number: 40095
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch: Make some internal decode methods protected.

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



Change subject: arch: Make some internal decode methods protected.
..

arch: Make some internal decode methods protected.

These methods aren't used outside of the decoder and the decode cache,
so they don't need to be public.

Change-Id: Ifdaf318995f1bb0a75b390bd1c5fde1211c66e62
---
M src/arch/arm/decoder.hh
M src/arch/mips/decoder.hh
M src/arch/power/decoder.hh
M src/arch/riscv/decoder.hh
M src/arch/sparc/decoder.hh
M src/arch/x86/decoder.hh
6 files changed, 53 insertions(+), 48 deletions(-)



diff --git a/src/arch/arm/decoder.hh b/src/arch/arm/decoder.hh
index 1f14328..53e3f76 100644
--- a/src/arch/arm/decoder.hh
+++ b/src/arch/arm/decoder.hh
@@ -82,6 +82,7 @@

 /// A cache of decoded instruction objects.
 static GenericISA::BasicDecodeCache defaultCache;
+friend class GenericISA::BasicDecodeCache;

 /**
  * Pre-decode an instruction from the current state of the
@@ -95,6 +96,38 @@
  */
 void consumeBytes(int numBytes);

+/**
+ * Decode a machine instruction without calling the cache.
+ *
+ * @note The implementation of this method is generated by the ISA
+ * parser script.
+ *
+ * @warn This method takes a pre-decoded instruction as its
+ * argument. It should typically not be called directly.
+ *
+ * @param mach_inst The binary instruction to decode.
+ * @retval A pointer to the corresponding StaticInst object.
+ */
+StaticInstPtr decodeInst(ExtMachInst mach_inst);
+
+/**
+ * Decode a pre-decoded machine instruction.
+ *
+ * @warn This method takes a pre-decoded instruction as its
+ * argument. It should typically not be called directly.
+ *
+ * @param mach_inst A pre-decoded instruction
+ * @retval A pointer to the corresponding StaticInst object.
+ */
+StaticInstPtr
+decode(ExtMachInst mach_inst, Addr addr)
+{
+StaticInstPtr si = defaultCache.decode(this, mach_inst, addr);
+DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
+si->getName(), mach_inst);
+return si;
+}
+
   public: // Decoder API
 Decoder(ISA* isa = nullptr);

@@ -162,38 +195,6 @@
 StaticInstPtr decode(ArmISA::PCState );

 /**
- * Decode a pre-decoded machine instruction.
- *
- * @warn This method takes a pre-decoded instruction as its
- * argument. It should typically not be called directly.
- *
- * @param mach_inst A pre-decoded instruction
- * @retval A pointer to the corresponding StaticInst object.
- */
-StaticInstPtr
-decode(ExtMachInst mach_inst, Addr addr)
-{
-StaticInstPtr si = defaultCache.decode(this, mach_inst, addr);
-DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
-si->getName(), mach_inst);
-return si;
-}
-
-/**
- * Decode a machine instruction without calling the cache.
- *
- * @note The implementation of this method is generated by the ISA
- * parser script.
- *
- * @warn This method takes a pre-decoded instruction as its
- * argument. It should typically not be called directly.
- *
- * @param mach_inst The binary instruction to decode.
- * @retval A pointer to the corresponding StaticInst object.
- */
-StaticInstPtr decodeInst(ExtMachInst mach_inst);
-
-/**
  * Take over the state from an old decoder when switching CPUs.
  *
  * @param old Decoder used in old CPU
diff --git a/src/arch/mips/decoder.hh b/src/arch/mips/decoder.hh
index 6e00bc3..969c152 100644
--- a/src/arch/mips/decoder.hh
+++ b/src/arch/mips/decoder.hh
@@ -89,8 +89,8 @@
   protected:
 /// A cache of decoded instruction objects.
 static GenericISA::BasicDecodeCache defaultCache;
+friend class GenericISA::BasicDecodeCache;

-  public:
 StaticInstPtr decodeInst(ExtMachInst mach_inst);

 /// Decode a machine instruction.
@@ -105,6 +105,7 @@
 return si;
 }

+  public:
 StaticInstPtr
 decode(MipsISA::PCState )
 {
diff --git a/src/arch/power/decoder.hh b/src/arch/power/decoder.hh
index ecbee72..e7dbd24 100644
--- a/src/arch/power/decoder.hh
+++ b/src/arch/power/decoder.hh
@@ -96,8 +96,8 @@
   protected:
 /// A cache of decoded instruction objects.
 static GenericISA::BasicDecodeCache defaultCache;
+friend class GenericISA::BasicDecodeCache;

-  public:
 StaticInstPtr decodeInst(ExtMachInst mach_inst);

 /// Decode a machine instruction.
@@ -112,6 +112,7 @@
 return si;
 }

+  public:
 StaticInstPtr
 decode(PowerISA::PCState )
 {
diff --git a/src/arch/riscv/decoder.hh b/src/arch/riscv/decoder.hh
index 23c3c38..173b4c7 100644
--- a/src/arch/riscv/decoder.hh
+++ b/src/arch/riscv/decoder.hh
@@ -56,6 +56,13 @@
 ExtMachInst emi;
 bool 

[gem5-dev] Change in gem5/gem5[develop]: arch,cpu: Move machInst into the arch specific StaticInst classes.

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



Change subject: arch,cpu: Move machInst into the arch specific StaticInst  
classes.

..

arch,cpu: Move machInst into the arch specific StaticInst classes.

This type is ISA specific. By moving it into the subclasses, it's still
available to everybody that needs it but avoids that ISA dependence in
the base StaticInst class.

Change-Id: I87ac4c6eded42287ef9ebaa4c4a5738482a2fc13
---
M src/arch/arm/insts/static_inst.hh
M src/arch/mips/isa/base.isa
M src/arch/power/insts/static_inst.hh
M src/arch/riscv/faults.cc
M src/arch/riscv/insts/static_inst.hh
M src/arch/sparc/insts/static_inst.hh
M src/arch/x86/faults.cc
M src/arch/x86/insts/static_inst.hh
M src/cpu/static_inst.cc
M src/cpu/static_inst.hh
10 files changed, 39 insertions(+), 27 deletions(-)



diff --git a/src/arch/arm/insts/static_inst.hh  
b/src/arch/arm/insts/static_inst.hh

index 908641c..1f4251a 100644
--- a/src/arch/arm/insts/static_inst.hh
+++ b/src/arch/arm/insts/static_inst.hh
@@ -143,10 +143,12 @@
 }
 }

+ExtMachInst machInst;
+
 // Constructor
 ArmStaticInst(const char *mnem, ExtMachInst _machInst,
   OpClass __opClass)
-: StaticInst(mnem, _machInst, __opClass)
+: StaticInst(mnem, __opClass), machInst(_machInst)
 {
 aarch64 = machInst.aarch64;
 if (bits(machInst, 28, 24) == 0x10)
diff --git a/src/arch/mips/isa/base.isa b/src/arch/mips/isa/base.isa
index cdd950c..0175bcc 100644
--- a/src/arch/mips/isa/base.isa
+++ b/src/arch/mips/isa/base.isa
@@ -42,10 +42,9 @@
 class MipsStaticInst : public StaticInst
 {
   protected:
-
 // Constructor
 MipsStaticInst(const char *mnem, MachInst _machInst, OpClass  
__opClass)

-: StaticInst(mnem, _machInst, __opClass)
+: StaticInst(mnem, __opClass), machInst(_machInst)
 {
 }

@@ -57,6 +56,8 @@
 Addr pc, const Loader::SymbolTable *symtab) const override;

   public:
+ExtMachInst machInst;
+
 void
 advancePC(MipsISA::PCState ) const override
 {
diff --git a/src/arch/power/insts/static_inst.hh  
b/src/arch/power/insts/static_inst.hh

index dc53bc1..403d358 100644
--- a/src/arch/power/insts/static_inst.hh
+++ b/src/arch/power/insts/static_inst.hh
@@ -38,10 +38,11 @@
 class PowerStaticInst : public StaticInst
 {
   protected:
+ExtMachInst machInst;

 // Constructor
-PowerStaticInst(const char *mnem, MachInst _machInst, OpClass  
__opClass)

-: StaticInst(mnem, _machInst, __opClass)
+PowerStaticInst(const char *mnem, ExtMachInst _machInst, OpClass  
__opClass)

+: StaticInst(mnem, __opClass), machInst(_machInst)
 {
 }

diff --git a/src/arch/riscv/faults.cc b/src/arch/riscv/faults.cc
index 5ac2a3c..8c273f2 100644
--- a/src/arch/riscv/faults.cc
+++ b/src/arch/riscv/faults.cc
@@ -32,6 +32,7 @@
 #include "arch/riscv/faults.hh"

 #include "arch/riscv/fs_workload.hh"
+#include "arch/riscv/insts/static_inst.hh"
 #include "arch/riscv/isa.hh"
 #include "arch/riscv/registers.hh"
 #include "arch/riscv/utility.hh"
@@ -163,14 +164,16 @@
 void
 UnknownInstFault::invokeSE(ThreadContext *tc, const StaticInstPtr )
 {
-panic("Unknown instruction 0x%08x at pc 0x%016llx", inst->machInst,
+auto *rsi = static_cast(inst.get());
+panic("Unknown instruction 0x%08x at pc 0x%016llx", rsi->machInst,
 tc->pcState().pc());
 }

 void
 IllegalInstFault::invokeSE(ThreadContext *tc, const StaticInstPtr )
 {
-panic("Illegal instruction 0x%08x at pc 0x%016llx: %s", inst->machInst,
+auto *rsi = static_cast(inst.get());
+panic("Illegal instruction 0x%08x at pc 0x%016llx: %s", rsi->machInst,
 tc->pcState().pc(), reason.c_str());
 }

diff --git a/src/arch/riscv/insts/static_inst.hh  
b/src/arch/riscv/insts/static_inst.hh

index 149e847..1c57cc7 100644
--- a/src/arch/riscv/insts/static_inst.hh
+++ b/src/arch/riscv/insts/static_inst.hh
@@ -46,9 +46,14 @@
 class RiscvStaticInst : public StaticInst
 {
   protected:
-using StaticInst::StaticInst;
+RiscvStaticInst(const char *_mnemonic, ExtMachInst _machInst,
+OpClass __opClass) :
+StaticInst(_mnemonic, __opClass), machInst(_machInst)
+{}

   public:
+ExtMachInst machInst;
+
 void advancePC(PCState ) const override { pc.advance(); }

 size_t
diff --git a/src/arch/sparc/insts/static_inst.hh  
b/src/arch/sparc/insts/static_inst.hh

index 0237c98..43d8d3d 100644
--- a/src/arch/sparc/insts/static_inst.hh
+++ b/src/arch/sparc/insts/static_inst.hh
@@ -87,7 +87,12 @@
 class SparcStaticInst : public StaticInst
 {
   protected:
-using StaticInst::StaticInst;
+ExtMachInst machInst;
+
+SparcStaticInst(const char *_mnemonic, ExtMachInst _machInst,
+OpClass __opClass) :
+StaticInst(_mnemonic, 

[gem5-dev] Change in gem5/gem5[develop]: arch: Stop using switching header files in ISA specific files.

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



Change subject: arch: Stop using switching header files in ISA specific  
files.

..

arch: Stop using switching header files in ISA specific files.

We know what ISA we want, we don't need to use the indirection.

Change-Id: I57eb2737bb4d9abb562b857ad2c3238c641199d2
---
M src/arch/arm/insts/tme64ruby.cc
M src/arch/arm/kvm/arm_cpu.cc
M src/arch/mips/locked_mem.hh
M src/arch/null/registers.hh
M src/arch/power/decoder.hh
M src/arch/riscv/locked_mem.hh
M src/arch/sparc/decoder.hh
7 files changed, 7 insertions(+), 7 deletions(-)



diff --git a/src/arch/arm/insts/tme64ruby.cc  
b/src/arch/arm/insts/tme64ruby.cc

index f8d9481..5e22deb 100644
--- a/src/arch/arm/insts/tme64ruby.cc
+++ b/src/arch/arm/insts/tme64ruby.cc
@@ -38,9 +38,9 @@
 #include "arch/arm/faults.hh"
 #include "arch/arm/htm.hh"
 #include "arch/arm/insts/tme64.hh"
+#include "arch/arm/locked_mem.hh"
 #include "arch/arm/registers.hh"
 #include "arch/generic/memhelpers.hh"
-#include "arch/locked_mem.hh"
 #include "debug/ArmTme.hh"
 #include "mem/packet_access.hh"
 #include "mem/request.hh"
diff --git a/src/arch/arm/kvm/arm_cpu.cc b/src/arch/arm/kvm/arm_cpu.cc
index f674c7e..e827c22 100644
--- a/src/arch/arm/kvm/arm_cpu.cc
+++ b/src/arch/arm/kvm/arm_cpu.cc
@@ -44,7 +44,7 @@
 #include 

 #include "arch/arm/interrupts.hh"
-#include "arch/registers.hh"
+#include "arch/arm/registers.hh"
 #include "cpu/kvm/base.hh"
 #include "debug/Kvm.hh"
 #include "debug/KvmContext.hh"
diff --git a/src/arch/mips/locked_mem.hh b/src/arch/mips/locked_mem.hh
index 153a991..73180af 100644
--- a/src/arch/mips/locked_mem.hh
+++ b/src/arch/mips/locked_mem.hh
@@ -47,7 +47,7 @@
  * ISA-specific helper functions for locked memory accesses.
  */

-#include "arch/registers.hh"
+#include "arch/mips/registers.hh"
 #include "base/logging.hh"
 #include "base/trace.hh"
 #include "cpu/base.hh"
diff --git a/src/arch/null/registers.hh b/src/arch/null/registers.hh
index db02afc..3e96472 100644
--- a/src/arch/null/registers.hh
+++ b/src/arch/null/registers.hh
@@ -40,7 +40,7 @@

 #include "arch/generic/vec_pred_reg.hh"
 #include "arch/generic/vec_reg.hh"
-#include "arch/types.hh"
+#include "arch/null/types.hh"
 #include "base/types.hh"

 namespace NullISA {
diff --git a/src/arch/power/decoder.hh b/src/arch/power/decoder.hh
index e7dbd24..bd32614 100644
--- a/src/arch/power/decoder.hh
+++ b/src/arch/power/decoder.hh
@@ -31,7 +31,7 @@

 #include "arch/generic/decode_cache.hh"
 #include "arch/generic/decoder.hh"
-#include "arch/types.hh"
+#include "arch/power/types.hh"
 #include "cpu/static_inst.hh"
 #include "debug/Decode.hh"

diff --git a/src/arch/riscv/locked_mem.hh b/src/arch/riscv/locked_mem.hh
index 10d1839..3a95780 100644
--- a/src/arch/riscv/locked_mem.hh
+++ b/src/arch/riscv/locked_mem.hh
@@ -49,7 +49,7 @@
 #include 
 #include 

-#include "arch/registers.hh"
+#include "arch/riscv/registers.hh"
 #include "base/logging.hh"
 #include "base/trace.hh"
 #include "cpu/base.hh"
diff --git a/src/arch/sparc/decoder.hh b/src/arch/sparc/decoder.hh
index 0c9f719..340733d 100644
--- a/src/arch/sparc/decoder.hh
+++ b/src/arch/sparc/decoder.hh
@@ -32,7 +32,7 @@
 #include "arch/generic/decode_cache.hh"
 #include "arch/generic/decoder.hh"
 #include "arch/sparc/registers.hh"
-#include "arch/types.hh"
+#include "arch/sparc/types.hh"
 #include "cpu/static_inst.hh"
 #include "debug/Decode.hh"


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40104
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: I57eb2737bb4d9abb562b857ad2c3238c641199d2
Gerrit-Change-Number: 40104
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch-arm,cpu: Use getEMI() in more places.

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



Change subject: arch-arm,cpu: Use getEMI() in more places.
..

arch-arm,cpu: Use getEMI() in more places.

Use that method to avoid reading the machInst.

Change-Id: I11434206c0b7a1aa3793aa46b5056ad60a64b01c
---
M src/arch/arm/tracers/tarmac_base.cc
M src/arch/arm/tracers/tarmac_parser.cc
M src/cpu/minor/dyn_inst.cc
3 files changed, 3 insertions(+), 3 deletions(-)



diff --git a/src/arch/arm/tracers/tarmac_base.cc  
b/src/arch/arm/tracers/tarmac_base.cc

index 7015cc2..e3364c4 100644
--- a/src/arch/arm/tracers/tarmac_base.cc
+++ b/src/arch/arm/tracers/tarmac_base.cc
@@ -64,7 +64,7 @@
 bool predicate)
 : taken(predicate) ,
   addr(pc.instAddr()) ,
-  opcode(staticInst->machInst & 0x),
+  opcode(staticInst->getEMI() & 0x),
   disassemble(staticInst->disassemble(addr)),
   isetstate(pcToISetState(pc)),
   mode(MODE_USER)
diff --git a/src/arch/arm/tracers/tarmac_parser.cc  
b/src/arch/arm/tracers/tarmac_parser.cc

index 3325342..db9c7e1 100644
--- a/src/arch/arm/tracers/tarmac_parser.cc
+++ b/src/arch/arm/tracers/tarmac_parser.cc
@@ -951,7 +951,7 @@
 outs << "\nMismatch between gem5 and TARMAC trace @ " << std::dec
  << curTick() << " ticks\n"
  << "[seq_num: " << std::dec << instRecord.seq_num
- << ", opcode: 0x" << std::hex << (staticInst->machInst &  
0x)
+ << ", opcode: 0x" << std::hex << (staticInst->getEMI() &  
0x)

  << ", PC: 0x" << pc.pc()
  << ", disasm: " <<  staticInst->disassemble(pc.pc()) << "]"
  << std::endl;
diff --git a/src/cpu/minor/dyn_inst.cc b/src/cpu/minor/dyn_inst.cc
index 1b43fc8..5a08da8 100644
--- a/src/cpu/minor/dyn_inst.cc
+++ b/src/cpu/minor/dyn_inst.cc
@@ -214,7 +214,7 @@
 regs_str << ',';
 }

-ccprintf(regs_str, " extMachInst=%160x", staticInst->machInst);
+ccprintf(regs_str, " extMachInst=%160x", staticInst->getEMI());
 }

 std::ostringstream flags;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40100
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: I11434206c0b7a1aa3793aa46b5056ad60a64b01c
Gerrit-Change-Number: 40100
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Fix style in decoder.hh.

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



Change subject: arch-arm: Fix style in decoder.hh.
..

arch-arm: Fix style in decoder.hh.

Change-Id: I45cf1fefc6145393abec2de12e74816c0c8ac0e9
---
M src/arch/arm/decoder.hh
1 file changed, 6 insertions(+), 3 deletions(-)



diff --git a/src/arch/arm/decoder.hh b/src/arch/arm/decoder.hh
index 598e865..fa5b15e 100644
--- a/src/arch/arm/decoder.hh
+++ b/src/arch/arm/decoder.hh
@@ -169,7 +169,8 @@
  * @param mach_inst A pre-decoded instruction
  * @retval A pointer to the corresponding StaticInst object.
  */
-StaticInstPtr decode(ExtMachInst mach_inst, Addr addr)
+StaticInstPtr
+decode(ExtMachInst mach_inst, Addr addr)
 {
 return defaultCache.decode(this, mach_inst, addr);
 }
@@ -197,13 +198,15 @@


   public: // ARM-specific decoder state manipulation
-void setContext(FPSCR fpscr)
+void
+setContext(FPSCR fpscr)
 {
 fpscrLen = fpscr.len;
 fpscrStride = fpscr.stride;
 }

-void setSveLen(uint8_t len)
+void
+setSveLen(uint8_t len)
 {
 sveLen = len;
 }

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40096
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: I45cf1fefc6145393abec2de12e74816c0c8ac0e9
Gerrit-Change-Number: 40096
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch: Templatize the BasicDecodeCache.

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



Change subject: arch: Templatize the BasicDecodeCache.
..

arch: Templatize the BasicDecodeCache.

While the arch/generic directory is in arch/, it still shouldn't assume
any particular ISA. This change templatizes away the ISA specific types
so it can be used in multiple ISAs at a time.

Change-Id: I1abb4f5081a0a25f743be786ad8e7e3d55cfc67a
---
M src/arch/arm/decoder.cc
M src/arch/arm/decoder.hh
M src/arch/generic/SConscript
D src/arch/generic/decode_cache.cc
M src/arch/generic/decode_cache.hh
M src/arch/mips/decoder.cc
M src/arch/mips/decoder.hh
M src/arch/power/decoder.cc
M src/arch/power/decoder.hh
M src/arch/sparc/decoder.cc
M src/arch/sparc/decoder.hh
11 files changed, 36 insertions(+), 78 deletions(-)



diff --git a/src/arch/arm/decoder.cc b/src/arch/arm/decoder.cc
index d7de6a2..f45849c 100644
--- a/src/arch/arm/decoder.cc
+++ b/src/arch/arm/decoder.cc
@@ -50,7 +50,7 @@
 namespace ArmISA
 {

-GenericISA::BasicDecodeCache Decoder::defaultCache;
+GenericISA::BasicDecodeCache Decoder::defaultCache;

 Decoder::Decoder(ISA* isa)
 : data(0), fpscrLen(0), fpscrStride(0),
diff --git a/src/arch/arm/decoder.hh b/src/arch/arm/decoder.hh
index fa5b15e..4f8e71a 100644
--- a/src/arch/arm/decoder.hh
+++ b/src/arch/arm/decoder.hh
@@ -80,7 +80,7 @@
 Enums::DecoderFlavor decoderFlavor;

 /// A cache of decoded instruction objects.
-static GenericISA::BasicDecodeCache defaultCache;
+static GenericISA::BasicDecodeCache defaultCache;

 /**
  * Pre-decode an instruction from the current state of the
diff --git a/src/arch/generic/SConscript b/src/arch/generic/SConscript
index 60b24d0..3ad4878 100644
--- a/src/arch/generic/SConscript
+++ b/src/arch/generic/SConscript
@@ -52,5 +52,4 @@
 if env['TARGET_ISA'] == 'null':
 Return()

-Source('decode_cache.cc')
 Source('decoder.cc')
diff --git a/src/arch/generic/decode_cache.cc  
b/src/arch/generic/decode_cache.cc

deleted file mode 100644
index 341cb70..000
--- a/src/arch/generic/decode_cache.cc
+++ /dev/null
@@ -1,58 +0,0 @@
-/*
- * Copyright (c) 2011-2012 Google
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met: redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer;
- * redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution;
- * neither the name of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include "arch/generic/decode_cache.hh"
-
-#include "arch/decoder.hh"
-#include "arch/types.hh"
-#include "config/the_isa.hh"
-#include "cpu/static_inst.hh"
-
-namespace GenericISA
-{
-
-StaticInstPtr
-BasicDecodeCache::decode(TheISA::Decoder *decoder,
-TheISA::ExtMachInst mach_inst, Addr addr)
-{
-StaticInstPtr  = decodePages.lookup(addr);
-if (si && (si->machInst == mach_inst))
-return si;
-
-auto iter = instMap.find(mach_inst);
-if (iter != instMap.end()) {
-si = iter->second;
-return si;
-}
-
-si = decoder->decodeInst(mach_inst);
-instMap[mach_inst] = si;
-return si;
-}
-
-} // namespace GenericISA
diff --git a/src/arch/generic/decode_cache.hh  
b/src/arch/generic/decode_cache.hh

index 564cb3e..84f90ca 100644
--- a/src/arch/generic/decode_cache.hh
+++ b/src/arch/generic/decode_cache.hh
@@ -29,31 +29,48 @@
 #ifndef __ARCH_GENERIC_DECODE_CACHE_HH__
 #define __ARCH_GENERIC_DECODE_CACHE_HH__

-#include "arch/types.hh"
-#include "config/the_isa.hh"
+#include "base/types.hh"
 #include "cpu/decode_cache.hh"
 #include "cpu/static_inst_fwd.hh"

-namespace TheISA
-{
-class Decoder;
-}
-
 namespace GenericISA
 {

+template 
 class 

[gem5-dev] Change in gem5/gem5[develop]: arch-arm,cpu: Introduce a getEMI virtual method on StaticInst.

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



Change subject: arch-arm,cpu: Introduce a getEMI virtual method on  
StaticInst.

..

arch-arm,cpu: Introduce a getEMI virtual method on StaticInst.

This takes the place of direct access to the machInst field as used in
the MinorCPU model which makes the incorrect assumption that it can
arbitrarily treat the ExtMachInst as an integer, and that masking in a
certain way can meaningfully classify what the instruction will do.

Because that assumption is not correct in general, that had been
ifdef-ed out in most ISAs except ARM, and for the other ISAs the value
was simply set to zero.

Change-Id: I8ac05e65475edc3ccc044afdff09490e2c05ba07
---
M src/arch/arm/insts/static_inst.hh
M src/cpu/minor/func_unit.cc
M src/cpu/static_inst.hh
3 files changed, 10 insertions(+), 7 deletions(-)



diff --git a/src/arch/arm/insts/static_inst.hh  
b/src/arch/arm/insts/static_inst.hh

index e101d93..908641c 100644
--- a/src/arch/arm/insts/static_inst.hh
+++ b/src/arch/arm/insts/static_inst.hh
@@ -197,6 +197,8 @@
 pcState.advance();
 }

+uint64_t getEMI() const override { return machInst; }
+
 std::string generateDisassembly(
 Addr pc, const Loader::SymbolTable *symtab) const override;

diff --git a/src/cpu/minor/func_unit.cc b/src/cpu/minor/func_unit.cc
index 58f3a1e..8c5e3a6 100644
--- a/src/cpu/minor/func_unit.cc
+++ b/src/cpu/minor/func_unit.cc
@@ -171,13 +171,12 @@
 MinorFUTiming *
 FUPipeline::findTiming(const StaticInstPtr )
 {
-#if THE_ISA == ARM_ISA
-/* This should work for any ISA with a POD mach_inst */
-TheISA::ExtMachInst mach_inst = inst->machInst;
-#else
-/* Just allow extra decode based on op classes */
-uint64_t mach_inst = 0;
-#endif
+/*
+ * This will only work on ISAs with an instruction format with a fixed  
size
+ * which can be categorized using bit masks. This is really only  
supported

+ * on ARM and is a bit of a hack.
+ */
+uint64_t mach_inst = inst->getEMI();

 const std::vector  =
 description.timings;
diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh
index 09f171f..b2cd508 100644
--- a/src/cpu/static_inst.hh
+++ b/src/cpu/static_inst.hh
@@ -258,6 +258,8 @@
 /// The binary machine instruction.
 const TheISA::ExtMachInst machInst;

+virtual uint64_t getEMI() const { return 0; }
+
   protected:

 /**

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40098
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: I8ac05e65475edc3ccc044afdff09490e2c05ba07
Gerrit-Change-Number: 40098
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch,cpu: Move a Decode DPRINTF into the arch Decoder classes.

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



Change subject: arch,cpu: Move a Decode DPRINTF into the arch Decoder  
classes.

..

arch,cpu: Move a Decode DPRINTF into the arch Decoder classes.

This DPRINTF accesses the ExtMachInst typed machInst member of the
StaticInst class, and so is ISA dependent. Move the DPRINTF to where the
instructions are actually decoded where that type doesn't have to be
disambiguated.

Also, this change makes this DPRINTF more accurate, since microops are
not really "decoded" when they are extracted from a macroop. The process
of unpacking them to feed into the rest of the CPU should be fairly
trivial, so really they're just being retrieved. With the DPRINTF in
this new position, it will only trigger when an instruction is actually
decoded from memory.

Change-Id: I14145165b93bb004057a729fa7909cd2d3d34d29
---
M src/arch/arm/decoder.hh
M src/arch/mips/decoder.hh
M src/arch/power/decoder.hh
M src/arch/riscv/decoder.cc
M src/arch/sparc/decoder.hh
M src/arch/x86/decoder.cc
M src/cpu/simple/base.cc
7 files changed, 41 insertions(+), 16 deletions(-)



diff --git a/src/arch/arm/decoder.hh b/src/arch/arm/decoder.hh
index 4f8e71a..1f14328 100644
--- a/src/arch/arm/decoder.hh
+++ b/src/arch/arm/decoder.hh
@@ -49,6 +49,7 @@
 #include "arch/generic/decoder.hh"
 #include "base/types.hh"
 #include "cpu/static_inst.hh"
+#include "debug/Decode.hh"
 #include "enums/DecoderFlavor.hh"

 namespace ArmISA
@@ -172,7 +173,10 @@
 StaticInstPtr
 decode(ExtMachInst mach_inst, Addr addr)
 {
-return defaultCache.decode(this, mach_inst, addr);
+StaticInstPtr si = defaultCache.decode(this, mach_inst, addr);
+DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
+si->getName(), mach_inst);
+return si;
 }

 /**
diff --git a/src/arch/mips/decoder.hh b/src/arch/mips/decoder.hh
index 9c6ae18..6e00bc3 100644
--- a/src/arch/mips/decoder.hh
+++ b/src/arch/mips/decoder.hh
@@ -35,6 +35,7 @@
 #include "base/logging.hh"
 #include "base/types.hh"
 #include "cpu/static_inst.hh"
+#include "debug/Decode.hh"

 namespace MipsISA
 {
@@ -98,7 +99,10 @@
 StaticInstPtr
 decode(ExtMachInst mach_inst, Addr addr)
 {
-return defaultCache.decode(this, mach_inst, addr);
+StaticInstPtr si = defaultCache.decode(this, mach_inst, addr);
+DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
+si->getName(), mach_inst);
+return si;
 }

 StaticInstPtr
diff --git a/src/arch/power/decoder.hh b/src/arch/power/decoder.hh
index f75450a..ecbee72 100644
--- a/src/arch/power/decoder.hh
+++ b/src/arch/power/decoder.hh
@@ -33,6 +33,7 @@
 #include "arch/generic/decoder.hh"
 #include "arch/types.hh"
 #include "cpu/static_inst.hh"
+#include "debug/Decode.hh"

 namespace PowerISA
 {
@@ -105,7 +106,10 @@
 StaticInstPtr
 decode(ExtMachInst mach_inst, Addr addr)
 {
-return defaultCache.decode(this, mach_inst, addr);
+StaticInstPtr si = defaultCache.decode(this, mach_inst, addr);
+DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
+si->getName(), mach_inst);
+return si;
 }

 StaticInstPtr
diff --git a/src/arch/riscv/decoder.cc b/src/arch/riscv/decoder.cc
index a117991..2e2474a 100644
--- a/src/arch/riscv/decoder.cc
+++ b/src/arch/riscv/decoder.cc
@@ -81,13 +81,18 @@
 {
 DPRINTF(Decode, "Decoding instruction 0x%08x at address %#x\n",
 mach_inst, addr);
-if (instMap.find(mach_inst) != instMap.end())
-return instMap[mach_inst];
-else {
+
+StaticInstPtr si;
+if (instMap.find(mach_inst) != instMap.end()) {
+si = instMap[mach_inst];
+} else {
 StaticInstPtr si = decodeInst(mach_inst);
 instMap[mach_inst] = si;
-return si;
 }
+
+DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
+si->getName(), mach_inst);
+return si;
 }

 StaticInstPtr
diff --git a/src/arch/sparc/decoder.hh b/src/arch/sparc/decoder.hh
index 14889a4..523d264 100644
--- a/src/arch/sparc/decoder.hh
+++ b/src/arch/sparc/decoder.hh
@@ -34,6 +34,7 @@
 #include "arch/sparc/registers.hh"
 #include "arch/types.hh"
 #include "cpu/static_inst.hh"
+#include "debug/Decode.hh"

 namespace SparcISA
 {
@@ -112,7 +113,10 @@
 StaticInstPtr
 decode(ExtMachInst mach_inst, Addr addr)
 {
-return defaultCache.decode(this, mach_inst, addr);
+StaticInstPtr si = defaultCache.decode(this, mach_inst, addr);
+DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
+si->getName(), mach_inst);
+return si;
 }

 StaticInstPtr
diff --git a/src/arch/x86/decoder.cc b/src/arch/x86/decoder.cc
index 415c7b4..95b80a8 100644
--- a/src/arch/x86/decoder.cc
+++ b/src/arch/x86/decoder.cc
@@ -32,6 +32,7 @@
 #include 

[gem5-dev] Change in gem5/gem5[develop]: Fix package parser

2021-01-29 Thread Earl Ou (Gerrit) via gem5-dev
Earl Ou has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40075 )



Change subject: Fix package parser
..

Fix package parser

Change-Id: Id5124135b0dd4049ce6531d7bdbc562d33f4d299
---
M util/decode_packet_trace.py
1 file changed, 1 insertion(+), 1 deletion(-)



diff --git a/util/decode_packet_trace.py b/util/decode_packet_trace.py
index e3486e6..21d7f9a 100755
--- a/util/decode_packet_trace.py
+++ b/util/decode_packet_trace.py
@@ -63,7 +63,7 @@
 exit(-1)

 # Read the magic number in 4-byte Little Endian
-magic_number = proto_in.read(4)
+magic_number = proto_in.read(4).decode()

 if magic_number != "gem5":
 print("Unrecognized file", sys.argv[1])

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40075
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: Id5124135b0dd4049ce6531d7bdbc562d33f4d299
Gerrit-Change-Number: 40075
Gerrit-PatchSet: 1
Gerrit-Owner: Earl Ou 
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