[gem5-dev] Re: SimObject params() method
As I look at this more, I do see a few places where the params struct is set aside for later legitimately. There aren't tons of them, but I think it's muddy enough where I don't want to go in with a hack saw quite yet. Gabe On Thu, Oct 22, 2020 at 5:10 PM Gabe Black wrote: > Ok, I'll make Params typedefs and params() usage targets of opportunity. > > Gabe > > On Thu, Oct 22, 2020 at 8:26 AM Jason Lowe-Power via gem5-dev < > gem5-dev@gem5.org> wrote: > >> Hey Gabe, >> >> Thanks for bringing this up. I have also been bothered by the lack of >> consistency with how params are used. I can't think of an example of when >> you need to store the params object. I would be all for getting rid of the >> params() function and updating the documentation to say that it's best >> practice to *not* save the params struct after the constructor. If some >> object had a good reason to go against this best practice, that would be >> OK, and we wouldn't need to enforce any specific design or pattern on these >> exceptions. I would prefer to remove the params() function than add more >> template magic. >> >> On Thu, Oct 22, 2020 at 1:18 AM Gabe Black via gem5-dev < >> gem5-dev@gem5.org> wrote: >> >>> Hi folks. I'm looking at the SimObject header, and I see a few things in >>> there which are marked as part of the API and maybe shouldn't be. >>> Specifically I'm talking about the Params typedef, and the params() method. >>> There is also the _params member variable which I can see being a part of >>> the API since it can be used by other classes to make their own params() >>> function (more on that below), but the Params typedef is more or less an >>> implementation detail, and the params() method is essentially worthless >>> because it returns a SimObjectParams which doesn't have anything except the >>> object's name in it, and you can already get that with the name() method. >>> >> >> I agree. I think the typedef is useless and shouldn't be in the API. It's >> unfortunate that the API proposals didn't get more reviews. I think it's >> probably OK to just drop that from the API, but the params() function >> itself is going to need to be deprecated. >> >> >>> >>> *Params pattern* >>> >>> This gets to the whole Params params() pattern which is sporadically >>> implemented in some SimObjects in gem5. This pattern is that a given >>> SimObject subclass will define a Params typedef which corresponds to its >>> Params struct type, and then also define a params() method which returns >>> the _params from SimObject cast up to that type. >>> >>> The Params typedef itself primarily makes the definition of the >>> constructor a little more legible, since the FullFooTypeForTheArmISAParams >>> can be really verbose. >>> >> >> I think verbose is fine. I would vote to abolish all params typedefs. >> >> >>> >>> Storing the params struct itself could theoretically serve the purpose >>> of having a bunch of parameters and not having to create a member variable >>> for each one, spend a bunch of text copying values over in the constructor, >>> etc. I think most of the time this is unnecessary, but if an object has >>> tons of values in it for some reason this could make sense. >>> >> >> I don't think there are any examples of this in the codebase. I think in >> all cases the params data is copied into member variables. If there are >> cases where data isn't copied, I doubt it was with a strong reason in mind. >> The one exception to this might be Ruby, but it's an exception for all the >> wrong reasons ;). >> >> >>> >>> The params() method then makes the _params member available with the >>> appropriate type, so that all the FooParams members are accessible. It also >>> makes the params struct accessible outside the object which is used in a >>> place or two to read parameters without there needing to be a member in the >>> object, and probably some sort of accessor to read it. >>> >>> There are two main problems with this system. First, when used, it adds >>> a small but not trivial amount of boilerplate to any given class. Second, >>> it's very sporadically implemented. I think in a lot of places it's there >>> just because people aren't sure what it's for or if they need it, so they >>> just put one there regardless. I think I've done that in the past. >>> >>> *Alternative* >>> >>> The existence of the Params type and the params() method could be >>> partially eliminated by defining a templated params() method which took a >>> SimObject reference and/or pointer as its first parameter. It could then >>> figure out what Params struct went with that SimObject type using >>> typedef/template magic set up by the Params building code, and then perform >>> the appropriate cast. >>> >>> This has three downsides, two minor and one more serious. The minor one >>> is that when a class uses this method internally, it would have to do >>> something like params(this) instead of just params(). That's a fairly minor >>> difference and not that big a deal
[gem5-dev] Change in gem5/gem5[develop]: util: Fix an incorrect print statement in git pre-commit hook
Hoa Nguyen has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/36516 )
Change subject: util: Fix an incorrect print statement in git pre-commit
hook
..
util: Fix an incorrect print statement in git pre-commit hook
Change-Id: I13d0a705b6cfab654635380e2adbf36243344a62
Signed-off-by: Hoa Nguyen
---
M util/git-pre-commit.py
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/git-pre-commit.py b/util/git-pre-commit.py
index 7681b87..ece2467 100755
--- a/util/git-pre-commit.py
+++ b/util/git-pre-commit.py
@@ -114,6 +114,6 @@
"fixes for commit in\n"
"the following files: ", file=sys.stderr)
for f in staged_mismatch:
-print("\t%s".format(f), file=sys.stderr)
+print("\t{}".format(f), file=sys.stderr)
print("Please `git --add' them", file=sys.stderr)
sys.exit(1)
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/36516
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: I13d0a705b6cfab654635380e2adbf36243344a62
Gerrit-Change-Number: 36516
Gerrit-PatchSet: 1
Gerrit-Owner: Hoa Nguyen
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: scons: Don't check for Python 2
Andreas Sandberg has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/36535 )
Change subject: scons: Don't check for Python 2
..
scons: Don't check for Python 2
The build system will now refuse to build gem5 if Python 2.x is
detected. Remove Python 2 specific python-config variants from the
list of candidates we try.
Change-Id: Id59be4a2969ce180848e5df02afdfb4a5b8125c1
Signed-off-by: Andreas Sandberg
---
M SConstruct
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/SConstruct b/SConstruct
index 3b157f0..8005a98 100755
--- a/SConstruct
+++ b/SConstruct
@@ -238,7 +238,7 @@
('MARSHAL_CCFLAGS_EXTRA', 'Extra C and C++ marshal compiler
flags', ''),
('MARSHAL_LDFLAGS_EXTRA', 'Extra marshal linker flags', ''),
('PYTHON_CONFIG', 'Python config binary to use',
-
[ 'python3-config', 'python-config', 'python2.7-config', 'python2-config']
+ [ 'python3-config', 'python-config']
),
('PROTOC', 'protoc tool', environ.get('PROTOC', 'protoc')),
('BATCH', 'Use batch pool for build and tests', False),
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/36535
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: Id59be4a2969ce180848e5df02afdfb4a5b8125c1
Gerrit-Change-Number: 36535
Gerrit-PatchSet: 1
Gerrit-Owner: Andreas Sandberg
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]: misc: Replace enable_if<>::type with enable_if_t<>.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/36477 )
Change subject: misc: Replace enable_if<>::type with enable_if_t<>.
..
misc: Replace enable_if<>::type with enable_if_t<>.
This new abreviated form was added for C++14. Now that we're using that
version of the standard, we can move over to it.
Change-Id: Ia291d2b1e73e503c37593b1e1c4c1b3011abc63b
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/36477
Reviewed-by: Daniel Carvalho
Maintainer: Gabe Black
Tested-by: kokoro
---
M src/arch/arm/aapcs32.hh
M src/arch/arm/aapcs64.hh
M src/arch/arm/freebsd/process.hh
M src/arch/arm/linux/process.hh
M src/arch/arm/process.hh
M src/arch/arm/semihosting.hh
M src/arch/gcn3/operand.hh
M src/arch/generic/vec_pred_reg.hh
M src/arch/generic/vec_reg.hh
M src/arch/sparc/process.hh
M src/arch/x86/linux/linux.hh
M src/arch/x86/linux/se_workload.hh
M src/base/circular_queue.hh
M src/base/coroutine.hh
M src/base/intmath.hh
M src/base/random.hh
M src/base/refcnt.hh
M src/base/statistics.hh
M src/base/str.hh
M src/sim/guest_abi.test.cc
M src/sim/guest_abi/dispatch.hh
M src/sim/guest_abi/layout.hh
M src/sim/proxy_ptr.hh
M src/sim/syscall_abi.hh
24 files changed, 161 insertions(+), 172 deletions(-)
Approvals:
Daniel Carvalho: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/aapcs32.hh b/src/arch/arm/aapcs32.hh
index 3cd2c50..a0f09b8 100644
--- a/src/arch/arm/aapcs32.hh
+++ b/src/arch/arm/aapcs32.hh
@@ -74,13 +74,13 @@
struct IsAapcs32Composite : public std::false_type {};
template
-struct IsAapcs32Composite::value ||
std::is_class::value ||
std::is_union::value) &&
// VarArgs is technically a composite type, but it's not a normal
argument.
!IsVarArgs::value
->::type> : public std::true_type
+>> : public std::true_type
{};
// Homogeneous Aggregates
@@ -130,9 +130,8 @@
*/
template
-struct Result-std::is_integral::value && (sizeof(Integer) <
sizeof(uint32_t))
->::type>
+struct Result+std::is_integral::value && (sizeof(Integer) <
sizeof(uint32_t))>>
{
static void
store(ThreadContext *tc, const Integer &i)
@@ -144,9 +143,8 @@
};
template
-struct Result-std::is_integral::value && (sizeof(Integer) ==
sizeof(uint32_t))
->::type>
+struct Result+std::is_integral::value && (sizeof(Integer) ==
sizeof(uint32_t))>>
{
static void
store(ThreadContext *tc, const Integer &i)
@@ -156,9 +154,8 @@
};
template
-struct Result-std::is_integral::value && (sizeof(Integer) ==
sizeof(uint64_t))
->::type>
+struct Result+std::is_integral::value && (sizeof(Integer) ==
sizeof(uint64_t))>>
{
static void
store(ThreadContext *tc, const Integer &i)
@@ -176,9 +173,9 @@
};
template
-struct Argument std::is_integral::value && (sizeof(Integer) <=
sizeof(uint32_t))
->::type> : public Aapcs32ArgumentBase
+>> : public Aapcs32ArgumentBase
{
static Integer
get(ThreadContext *tc, Aapcs32::State &state)
@@ -195,9 +192,9 @@
};
template
-struct Argument std::is_integral::value && (sizeof(Integer) >
sizeof(uint32_t))
->::type> : public Aapcs32ArgumentBase
+>> : public Aapcs32ArgumentBase
{
static Integer
get(ThreadContext *tc, Aapcs32::State &state)
@@ -236,8 +233,8 @@
*/
template
-struct Result::value>::type>
+struct Result::value>>
{
static void
store(ThreadContext *tc, const Float &f, Aapcs32::State &state)
@@ -248,8 +245,8 @@
};
template
-struct Argument-std::is_floating_point::value>::type> : public
Aapcs32ArgumentBase
+struct Argument::value>> : public Aapcs32ArgumentBase
{
static Float
get(ThreadContext *tc, Aapcs32::State &state)
@@ -270,8 +267,8 @@
*/
template
-struct Result::value>::type>
+struct Result::value>>
{
static void
store(ThreadContext *tc, const Composite &composite,
@@ -298,8 +295,8 @@
};
template
-struct Argument::value>::type> :
+struct Argument::value>> :
public Aapcs32ArgumentBase
{
static Composite
@@ -444,13 +441,13 @@
*/
template
-struct Result-std::is_integral::value>::type> : public ResultInteger>
+struct Result::value>> : public Result
{};
template
-struct Argument::value>::type> :
+struct Argument::value>> :
public Argument
{};
@@ -460,8 +457,8 @@
*/
template
-struct Result::value>::type>
+struct Result::value>>
{
static void
store(ThreadContext *tc, const Float &f, Aapcs32Vfp::State &state)
@@ -479,8 +476,8 @@
};
template
-struct Argument-std::is_floating_point::value>::type> : public
Aapcs32ArgumentBase
+struct Argument::value>> : public Aapcs32ArgumentBase
{
static Float
get(ThreadContext *tc, Aapcs32Vfp::State &state)
@@ -510,16 +507,16 @@
*/
template
-struct Result::value &&
-!IsAapcs32Homogeneous
[gem5-dev] Change in gem5/gem5[develop]: scons: Don't check for Python 2
Andreas Sandberg has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/36535 )
Change subject: scons: Don't check for Python 2
..
scons: Don't check for Python 2
The build system will now refuse to build gem5 if Python 2.x is
detected. Remove Python 2 specific python-config variants from the
list of candidates we try.
Change-Id: Id59be4a2969ce180848e5df02afdfb4a5b8125c1
Signed-off-by: Andreas Sandberg
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/36535
Reviewed-by: Gabe Black
Reviewed-by: Daniel Carvalho
Reviewed-by: Jason Lowe-Power
Maintainer: Gabe Black
Tested-by: kokoro
---
M SConstruct
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Daniel Carvalho: Looks good to me, but someone else must approve
Gabe Black: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/SConstruct b/SConstruct
index a057da3..5033344 100755
--- a/SConstruct
+++ b/SConstruct
@@ -238,7 +238,7 @@
('MARSHAL_CCFLAGS_EXTRA', 'Extra C and C++ marshal compiler
flags', ''),
('MARSHAL_LDFLAGS_EXTRA', 'Extra marshal linker flags', ''),
('PYTHON_CONFIG', 'Python config binary to use',
-
[ 'python3-config', 'python-config', 'python2.7-config', 'python2-config']
+ [ 'python3-config', 'python-config']
),
('PROTOC', 'protoc tool', environ.get('PROTOC', 'protoc')),
('BATCH', 'Use batch pool for build and tests', False),
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/36535
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: Id59be4a2969ce180848e5df02afdfb4a5b8125c1
Gerrit-Change-Number: 36535
Gerrit-PatchSet: 3
Gerrit-Owner: Andreas Sandberg
Gerrit-Assignee: Jason Lowe-Power
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Bobby R. Bruce
Gerrit-Reviewer: Daniel Carvalho
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: kokoro
Gerrit-CC: Jason Lowe-Power
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: scons: Test if binaries can embed the Python interpreter
Andreas Sandberg has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/36383 )
Change subject: scons: Test if binaries can embed the Python interpreter
..
scons: Test if binaries can embed the Python interpreter
Add some more stringent Python tests that ensure that we can link with
and run applications that embed Python. This is implemented by running
building a small c++ program that embeds Python using PyBind11. The
program is run by the build system and prints the version of the
Python interpreter. The version information is then used by the build
system to ensure that the installed version is supported.
Change-Id: I727e0832f171362f5506247c022bea365068a0f6
Signed-off-by: Andreas Sandberg
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/36383
Reviewed-by: Gabe Black
Reviewed-by: Daniel Carvalho
Maintainer: Gabe Black
Tested-by: kokoro
---
M SConstruct
1 file changed, 33 insertions(+), 3 deletions(-)
Approvals:
Daniel Carvalho: Looks good to me, but someone else must approve
Gabe Black: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/SConstruct b/SConstruct
index 8e7ec34..a057da3 100755
--- a/SConstruct
+++ b/SConstruct
@@ -575,6 +575,27 @@
context.Result(ret)
return ret
+def CheckPythonLib(context):
+context.Message('Checking Python version... ')
+ret = context.TryRun(r"""
+#include
+
+int
+main(int argc, char **argv) {
+pybind11::scoped_interpreter guard{};
+pybind11::exec(
+"import sys\n"
+"vi = sys.version_info\n"
+"sys.stdout.write('%i.%i.%i' % (vi.major, vi.minor,
vi.micro));\n");
+return 0;
+}
+""", extension=".cc")
+context.Result(ret[1] if ret[0] == 1 else 0)
+if ret[0] == 0:
+return None
+else:
+return tuple(map(int, ret[1].split(".")))
+
# Platform-specific configuration. Note again that we assume that all
# builds under a given build root run on the same host platform.
conf = Configure(main,
@@ -582,6 +603,7 @@
log_file = joinpath(build_root, 'scons_config.log'),
custom_tests = {
'CheckMember' : CheckMember,
+'CheckPythonLib' : CheckPythonLib,
})
# Check if we should compile a 64 bit binary on Mac OS X/Darwin
@@ -637,9 +659,6 @@
main['PYTHON_CONFIG'])
print("Info: Using Python config: %s" % (python_config, ))
-if python_config != 'python3-config':
-warning('python3-config could not be found.\n'
-'Future releases of gem5 will drop support for python2.')
py_includes = readCommand([python_config, '--includes'],
exception='').split()
@@ -697,6 +716,17 @@
marshal_env = main.Clone()
marshal_env.Append(CCFLAGS='$MARSHAL_CCFLAGS_EXTRA')
marshal_env.Append(LINKFLAGS='$MARSHAL_LDFLAGS_EXTRA')
+py_version = conf.CheckPythonLib()
+if not py_version:
+error("Can't find a working Python installation")
+
+# Found a working Python installation. Check if it meets minimum
+# requirements.
+if py_version[0] < 3 or \
+ (py_version[0] == 3 and py_version[1] < 6):
+error('Python version too old. Version 3.6 or newer is required.')
+elif py_version[0] > 3:
+warning('Python version too new. Python 3 expected.')
# On Solaris you need to use libsocket for socket ops
if not
conf.CheckLibWithHeader(None, 'sys/socket.h', 'C++', 'accept(0,0,0);'):
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/36383
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: I727e0832f171362f5506247c022bea365068a0f6
Gerrit-Change-Number: 36383
Gerrit-PatchSet: 4
Gerrit-Owner: Andreas Sandberg
Gerrit-Assignee: Bobby R. Bruce
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Ciro Santilli
Gerrit-Reviewer: Daniel Carvalho
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Giacomo Travaglini
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: kokoro
Gerrit-CC: Bobby R. Bruce
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Re: SimObject params() method
With the #include method you could have multiple params for the same
simobject. I know mid-file includes are rare, but, like goto, there are
use-cases that make sense. In this case, imo, it improves clarity.
You can just inherit from MySimObject::Params, and you aren't restricted to
doing it in any specific source file.
$cat main.cc
#include
#include
#include "MySimObject.hh"
struct FooParams : MySimObject::Params {
char foo;
};
int
main( int argc, char *argv[]){
FooParams p;
p.myint = 5;
p.foo = 'f';
MySimObject my_so_instance(&p);
std::cout << my_so_instance.myInt() << std::endl;
return EXIT_SUCCESS;
}
Ryan Gambord
On Thu, Oct 22, 2020 at 7:58 PM Gabe Black via gem5-dev
wrote:
> [This email originated from outside of OSU. Use caution with links and
> attachments.]
> Also that (like the other mechanisms) means that there can be at most one
> C++ class for a given Params class. It would be impossible to have
> FooParams -> SimObj and BarParams -> SimObj since there is only room for
> one Params in a class.
>
> That said, we could turn the FooParams and BarParams into Params::Foo and
> Params::Bar. That would be fairly trivial, and would mean we'd only have
> one name to dodge, Params, although that's an easier name to collide with
> accidentally.
>
> I think a step in the right direction would be to start putting all our
> gem5 stuff into a gem5 namespace. Namespaces are easy to fold away in
> context (using, opening the namespace and working within it), and then we
> would only have to worry about internal collisions which I think are a lot
> easier to manage.
>
> We could even start doing that now without breaking compatibility by
> putting things in a gem5 namespace and then putting a "using namespace
> ::gem5;" at the end to maintain compatibility until we decide to switch.
>
> Gabe
>
> On Thu, Oct 22, 2020 at 7:50 PM Gabe Black wrote:
>
>> That would work, but we don't have includes in the middle of a class like
>> that anywhere else in gem5 (that I can remember at least), which I think is
>> for the best :-).
>>
>> Gabe
>>
>> On Thu, Oct 22, 2020 at 6:28 PM Gambord, Ryan via gem5-dev <
>> gem5-dev@gem5.org> wrote:
>>
>>> @Gabe Black
>>> Here is an example of what this looks like. It requires slightly
>>> modifying the boilerplate code generated for params structs, and then
>>> moving the include from the top of the source file to inside the class.
>>> This brings the struct into the class namespace.
>>>
>>> This should work fine as long as pybind can work with a FQN of an
>>> object.
>>>
>>> MySimObject.hh
>>> #ifndef __MYSIMOBJECT_HH__
>>> #define __MYSIMOBJECT_HH__
>>>
>>> class MySimObject
>>> {
>>> public:
>>> #include "MySimObjectParams.hh"
>>>
>>> public:
>>> MySimObject(Params * p) : myint(p->myint) {}
>>>
>>> int & myInt() { return myint; }
>>>
>>> private:
>>> int myint;
>>> };
>>> #endif // __MYSIMOBJECT_HH__
>>>
>>> MySimObjectParams.hh
>>> // Generated automatically by SCons //
>>>
>>> struct Params {
>>> int myint;
>>> };
>>>
>>> main.cc
>>> #include
>>> #include
>>>
>>> #include "MySimObject.hh"
>>>
>>> int
>>> main( int argc, char *argv[]){
>>> MySimObject::Params p;
>>> p.myint = 5;
>>> MySimObject my_so_instance(&p);
>>> std::cout << my_so_instance.myInt() << std::endl;
>>> return EXIT_SUCCESS;
>>> }
>>>
>>> Ryan Gambord
>>>
>>>
>>>
>>>
>>> On Thu, Oct 22, 2020 at 5:20 PM Gabe Black via gem5-dev <
>>> gem5-dev@gem5.org> wrote:
>>>
[This email originated from outside of OSU. Use caution with links and
attachments.]
I definitely agree that we need to use more namespaces and be better
citizens in the global namespace.
See https://gem5.atlassian.net/browse/GEM5-730
Mechanically, I don't think we can define something like
Mem::Ruby::Network::BasicLink::Params since that would require adding a
Params type to the not yet defined BasicLink class. You can do that with
namespaces, but not with classes. We also couldn't add a params function to
the SimObject for the same reason. Python doesn't define the SimObject
class at all, even though it might look like it does because the python
objects and C++ objects often (but not always) share the same name. The
Python objects actually are the Params structs in C++, and then python uses
the create method to make the actual SimObjects. The real C++ SimObject
classes have no python analogue, although python has pointers to them and
can indirectly call methods on them once they've been instantiated. That's
how callbacks like init(), startup(), etc are handled.
Gabe
On Thu, Oct 22, 2020 at 10:25 AM Gambord, Ryan via gem5-dev <
gem5-dev@gem5.org> wrote:
> While we're thinking about the params implementations, I'd also like
> to propose that we stop cluttering the global namespace with
> collision-prone names:
>
> src/.../.../MySimObject.hh
> na
[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Reimplement TLB::flushAll
Giacomo Travaglini has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/35239 )
Change subject: arch-arm: Reimplement TLB::flushAll
..
arch-arm: Reimplement TLB::flushAll
flushAll is a non architectural flush command; this is not based on
flushAllSecurity anymore. flushAll should always flush stage1 and stage2,
whereas flushAllSecurity is checking for the current state
(vmid, and if we are in Hyp)
Change-Id: I6b81ebfba387e646f256ecbecb7b5ee720745358
Signed-off-by: Giacomo Travaglini
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/35239
Reviewed-by: Andreas Sandberg
Reviewed-by: Richard Cooper
Maintainer: Andreas Sandberg
Tested-by: kokoro
---
M src/arch/arm/tlb.cc
M src/arch/arm/tlb.hh
2 files changed, 30 insertions(+), 11 deletions(-)
Approvals:
Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
Richard Cooper: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/tlb.cc b/src/arch/arm/tlb.cc
index 0f9d7cd..6f8b46f 100644
--- a/src/arch/arm/tlb.cc
+++ b/src/arch/arm/tlb.cc
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010-2013, 2016-2019 ARM Limited
+ * Copyright (c) 2010-2013, 2016-2020 ARM Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -246,6 +246,29 @@
}
void
+TLB::flushAll()
+{
+DPRINTF(TLB, "Flushing all TLB entries\n");
+int x = 0;
+TlbEntry *te;
+while (x < size) {
+te = &table[x];
+
+DPRINTF(TLB, " - %s\n", te->print());
+te->valid = false;
+stats.flushedEntries++;
+++x;
+}
+
+stats.flushTlb++;
+
+// If there's a second stage TLB (and we're not it) then flush it as
well
+if (!isStage2) {
+stage2Tlb->flushAll();
+}
+}
+
+void
TLB::flushAllSecurity(bool secure_lookup, ExceptionLevel target_el,
bool ignore_el, bool in_host)
{
diff --git a/src/arch/arm/tlb.hh b/src/arch/arm/tlb.hh
index f839848..6363b7c 100644
--- a/src/arch/arm/tlb.hh
+++ b/src/arch/arm/tlb.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010-2013, 2016, 2019 ARM Limited
+ * Copyright (c) 2010-2013, 2016, 2019-2020 ARM Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -246,6 +246,11 @@
bool checkPAN(ThreadContext *tc, uint8_t ap, const RequestPtr &req,
Mode mode);
+/** Reset the entire TLB. Used for CPU switching to prevent stale
+ * translations after multiple switches
+ */
+void flushAll() override;
+
/** Reset the entire TLB
* @param secure_lookup if the operation affects the secure world
@@ -259,15 +264,6 @@
void flushAllNs(ExceptionLevel target_el, bool ignore_el = false);
-/** Reset the entire TLB. Used for CPU switching to prevent stale
- * translations after multiple switches
- */
-void flushAll() override
-{
-flushAllSecurity(false, EL0, true, false);
-flushAllSecurity(true, EL0, true, false);
-}
-
/** Remove any entries that match both a va and asn
* @param mva virtual address to flush
* @param asn contextid/asn to flush on match
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/35239
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: I6b81ebfba387e646f256ecbecb7b5ee720745358
Gerrit-Change-Number: 35239
Gerrit-PatchSet: 9
Gerrit-Owner: Giacomo Travaglini
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Giacomo Travaglini
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]: arch-arm: TLBIALL/TLBIASID/TLBIMVA base classes for I/D flavours
Giacomo Travaglini has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/35238 )
Change subject: arch-arm: TLBIALL/TLBIASID/TLBIMVA base classes for I/D
flavours
..
arch-arm: TLBIALL/TLBIASID/TLBIMVA base classes for I/D flavours
This will be exploited by the incoming patchset
Change-Id: Ic10a8d64910a04d4153b0f2abb133dfd56dbaf62
Signed-off-by: Giacomo Travaglini
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/35238
Tested-by: kokoro
Reviewed-by: Andreas Sandberg
Reviewed-by: Richard Cooper
Maintainer: Andreas Sandberg
---
M src/arch/arm/tlbi_op.hh
1 file changed, 13 insertions(+), 27 deletions(-)
Approvals:
Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
Richard Cooper: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/tlbi_op.hh b/src/arch/arm/tlbi_op.hh
index 8706d3d..511d70e 100644
--- a/src/arch/arm/tlbi_op.hh
+++ b/src/arch/arm/tlbi_op.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2018-2019 ARM Limited
+ * Copyright (c) 2018-2020 ARM Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -89,11 +89,11 @@
};
/** Instruction TLB Invalidate All */
-class ITLBIALL : public TLBIOp
+class ITLBIALL : public TLBIALL
{
public:
ITLBIALL(ExceptionLevel _targetEL, bool _secure)
- : TLBIOp(_targetEL, _secure)
+ : TLBIALL(_targetEL, _secure)
{}
void broadcast(ThreadContext *tc) = delete;
@@ -102,11 +102,11 @@
};
/** Data TLB Invalidate All */
-class DTLBIALL : public TLBIOp
+class DTLBIALL : public TLBIALL
{
public:
DTLBIALL(ExceptionLevel _targetEL, bool _secure)
- : TLBIOp(_targetEL, _secure)
+ : TLBIALL(_targetEL, _secure)
{}
void broadcast(ThreadContext *tc) = delete;
@@ -129,35 +129,29 @@
};
/** Instruction TLB Invalidate by ASID match */
-class ITLBIASID : public TLBIOp
+class ITLBIASID : public TLBIASID
{
public:
ITLBIASID(ExceptionLevel _targetEL, bool _secure, uint16_t _asid)
- : TLBIOp(_targetEL, _secure), asid(_asid)
+ : TLBIASID(_targetEL, _secure, _asid)
{}
void broadcast(ThreadContext *tc) = delete;
void operator()(ThreadContext* tc) override;
-
- protected:
-uint16_t asid;
};
/** Data TLB Invalidate by ASID match */
-class DTLBIASID : public TLBIOp
+class DTLBIASID : public TLBIASID
{
public:
DTLBIASID(ExceptionLevel _targetEL, bool _secure, uint16_t _asid)
- : TLBIOp(_targetEL, _secure), asid(_asid)
+ : TLBIASID(_targetEL, _secure, _asid)
{}
void broadcast(ThreadContext *tc) = delete;
void operator()(ThreadContext* tc) override;
-
- protected:
-uint16_t asid;
};
/** TLB Invalidate All, Non-Secure */
@@ -203,39 +197,31 @@
};
/** Instruction TLB Invalidate by VA */
-class ITLBIMVA : public TLBIOp
+class ITLBIMVA : public TLBIMVA
{
public:
ITLBIMVA(ExceptionLevel _targetEL, bool _secure,
Addr _addr, uint16_t _asid)
- : TLBIOp(_targetEL, _secure), addr(_addr), asid(_asid)
+ : TLBIMVA(_targetEL, _secure, _addr, _asid)
{}
void broadcast(ThreadContext *tc) = delete;
void operator()(ThreadContext* tc) override;
-
- protected:
-Addr addr;
-uint16_t asid;
};
/** Data TLB Invalidate by VA */
-class DTLBIMVA : public TLBIOp
+class DTLBIMVA : public TLBIMVA
{
public:
DTLBIMVA(ExceptionLevel _targetEL, bool _secure,
Addr _addr, uint16_t _asid)
- : TLBIOp(_targetEL, _secure), addr(_addr), asid(_asid)
+ : TLBIMVA(_targetEL, _secure, _addr, _asid)
{}
void broadcast(ThreadContext *tc) = delete;
void operator()(ThreadContext* tc) override;
-
- protected:
-Addr addr;
-uint16_t asid;
};
/** TLB Invalidate by Intermediate Physical Address */
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/35238
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: Ic10a8d64910a04d4153b0f2abb133dfd56dbaf62
Gerrit-Change-Number: 35238
Gerrit-PatchSet: 9
Gerrit-Owner: Giacomo Travaglini
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Giacomo Travaglini
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]: arch-arm: Rewrite the TLB flushing interface
Giacomo Travaglini has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/35240 )
Change subject: arch-arm: Rewrite the TLB flushing interface
..
arch-arm: Rewrite the TLB flushing interface
We are now using an overloaded flush method which has
different TLBI ops as arguments.
This is simplifying the interface and it is allowing us to
encode some state in the TLBIOp which will then be passed
to the TLB. This is a step towards making the TLB a stateless
cache of translations
Change-Id: Ic4fbae72dc3cfe756047148b1cf5f144298c8b08
Signed-off-by: Giacomo Travaglini
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/35240
Reviewed-by: Andreas Sandberg
Reviewed-by: Richard Cooper
Maintainer: Andreas Sandberg
Tested-by: kokoro
---
M src/arch/arm/mmu.cc
M src/arch/arm/mmu.hh
M src/arch/arm/tlb.cc
M src/arch/arm/tlb.hh
M src/arch/arm/tlbi_op.cc
M src/arch/arm/tlbi_op.hh
6 files changed, 111 insertions(+), 219 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/src/arch/arm/mmu.cc b/src/arch/arm/mmu.cc
index c3e487f..0895cee 100644
--- a/src/arch/arm/mmu.cc
+++ b/src/arch/arm/mmu.cc
@@ -36,6 +36,7 @@
*/
#include "arch/arm/mmu.hh"
+#include "arch/arm/tlbi_op.hh"
using namespace ArmISA;
@@ -66,84 +67,6 @@
}
}
-void
-MMU::flushAllSecurity(bool secure_lookup, ExceptionLevel target_el,
-TLBType type, bool ignore_el, bool in_host)
-{
-if (type & TLBType::I_TLBS) {
-getITBPtr()->flushAllSecurity(
-secure_lookup, target_el, ignore_el, in_host);
-}
-if (type & TLBType::D_TLBS) {
-getDTBPtr()->flushAllSecurity(
-secure_lookup, target_el, ignore_el, in_host);
-}
-}
-
-void
-MMU::flushAllNs(ExceptionLevel target_el, bool ignore_el,
-TLBType type)
-{
-if (type & TLBType::I_TLBS) {
-getITBPtr()->flushAllNs(target_el, ignore_el);
-}
-if (type & TLBType::D_TLBS) {
-getDTBPtr()->flushAllNs(target_el, ignore_el);
-}
-}
-
-void
-MMU::flushMvaAsid(Addr mva, uint64_t asn, bool secure_lookup,
-ExceptionLevel target_el, TLBType type,
-bool in_host)
-{
-if (type & TLBType::I_TLBS) {
-getITBPtr()->flushMvaAsid(
-mva, asn, secure_lookup, target_el, in_host);
-}
-if (type & TLBType::D_TLBS) {
-getDTBPtr()->flushMvaAsid(
-mva, asn, secure_lookup, target_el, in_host);
-}
-}
-
-void
-MMU::flushAsid(uint64_t asn, bool secure_lookup,
-ExceptionLevel target_el, TLBType type,
-bool in_host)
-{
-if (type & TLBType::I_TLBS) {
-getITBPtr()->flushAsid(asn, secure_lookup, target_el, in_host);
-}
-if (type & TLBType::D_TLBS) {
-getDTBPtr()->flushAsid(asn, secure_lookup, target_el, in_host);
-}
-}
-
-void
-MMU::flushMva(Addr mva, bool secure_lookup, ExceptionLevel target_el,
-TLBType type, bool in_host)
-{
-if (type & TLBType::I_TLBS) {
-getITBPtr()->flushMva(mva, secure_lookup, target_el, in_host);
-}
-if (type & TLBType::D_TLBS) {
-getDTBPtr()->flushMva(mva, secure_lookup, target_el, in_host);
-}
-}
-
-void
-MMU::flushIpaVmid(Addr ipa, bool secure_lookup, ExceptionLevel target_el,
-TLBType type)
-{
-if (type & TLBType::I_TLBS) {
-getITBPtr()->flushIpaVmid(ipa, secure_lookup, target_el);
-}
-if (type & TLBType::D_TLBS) {
-getDTBPtr()->flushIpaVmid(ipa, secure_lookup, target_el);
-}
-}
-
ArmISA::MMU *
ArmMMUParams::create() const
{
diff --git a/src/arch/arm/mmu.hh b/src/arch/arm/mmu.hh
index cc4684f..e5adb95 100644
--- a/src/arch/arm/mmu.hh
+++ b/src/arch/arm/mmu.hh
@@ -79,58 +79,33 @@
void invalidateMiscReg(TLBType type = ALL_TLBS);
-/** Reset the entire TLB
- * @param secure_lookup if the operation affects the secure world
- */
-void flushAllSecurity(bool secure_lookup, ExceptionLevel target_el,
- TLBType type = ALL_TLBS,
- bool ignore_el = false, bool in_host = false);
+template
+void
+flush(const OP &tlbi_op)
+{
+getITBPtr()->flush(tlbi_op);
+getDTBPtr()->flush(tlbi_op);
+}
-/** Remove all entries in the non secure world, depending on whether
they
- * were allocated in hyp mode or not
- */
-void flushAllNs(ExceptionLevel target_el, bool ignore_el = false,
-TLBType type = ALL_TLBS);
+template
+void
+iflush(const OP &tlbi_op)
+{
+getITBPtr()->flush(tlbi_op);
+}
-/** Remove any entries that match both a va and asn
- * @param mva virtual address to flush
- * @param asn contextid/asn to flush on match
- * @param secure_lookup if the operation affects the secure world
- */
-void flushMvaAsid(Addr mva, uint64_t asn, bool secure_lookup,
-Ex
[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Fix implementation of TLBI ALLEx instructions
Giacomo Travaglini has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/35241 )
Change subject: arch-arm: Fix implementation of TLBI ALLEx instructions
..
arch-arm: Fix implementation of TLBI ALLEx instructions
The TLBIALL op in gem5 was designed after the AArch32 TLBIALL instruction.
and was reused by the TLBI ALLEL1, ALLE2, ALLE3 logic.
This is not correct for the following reasons:
- TLBI ALLEx invalidates regardless of the VMID
- TLBI ALLEx (AArch64) is "target regime" oriented, whereas TLBIALL
(AArch32) is "current regime" oriented
TLBIALL has a different behaviour depending on the current exception
level: if issued at EL1 it will invalidate stage1 translations only; if
at EL2, it will invalidate stage2 translations as well.
TLBI ALLEx is more standard; every TLBI ALLE1 will invalidate stage1 and
stage2 translations. This is because the instruction is not executable
from the guest (EL1)
So for TLBIALL the condition for stage2 forwarding will be:
if (!isStage2 && isHyp) {
Whereas for TLBI ALLEx will be:
if (!isStage2 && target_el == EL1) {
Change-Id: I282f2cfaecbfc883e173770e5d2578b41055bb7a
Signed-off-by: Giacomo Travaglini
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/35241
Reviewed-by: Andreas Sandberg
Maintainer: Andreas Sandberg
Tested-by: kokoro
---
M src/arch/arm/isa.cc
M src/arch/arm/tlb.cc
M src/arch/arm/tlb.hh
M src/arch/arm/tlbi_op.cc
M src/arch/arm/tlbi_op.hh
5 files changed, 92 insertions(+), 7 deletions(-)
Approvals:
Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/isa.cc b/src/arch/arm/isa.cc
index d3f9c98..1c70a77 100644
--- a/src/arch/arm/isa.cc
+++ b/src/arch/arm/isa.cc
@@ -1701,7 +1701,7 @@
{
assert64();
-TLBIALL tlbiOp(EL3, true);
+TLBIALLEL tlbiOp(EL3, true);
tlbiOp(tc);
return;
}
@@ -1710,7 +1710,7 @@
{
assert64();
-TLBIALL tlbiOp(EL3, true);
+TLBIALLEL tlbiOp(EL3, true);
tlbiOp.broadcast(tc);
return;
}
@@ -1720,7 +1720,7 @@
assert64();
scr = readMiscReg(MISCREG_SCR);
-TLBIALL tlbiOp(EL2, haveSecurity && !scr.ns);
+TLBIALLEL tlbiOp(EL2, haveSecurity && !scr.ns);
tlbiOp(tc);
return;
}
@@ -1730,12 +1730,30 @@
assert64();
scr = readMiscReg(MISCREG_SCR);
-TLBIALL tlbiOp(EL2, haveSecurity && !scr.ns);
+TLBIALLEL tlbiOp(EL2, haveSecurity && !scr.ns);
tlbiOp.broadcast(tc);
return;
}
// AArch64 TLB Invalidate All, EL1
case MISCREG_TLBI_ALLE1:
+{
+assert64();
+scr = readMiscReg(MISCREG_SCR);
+
+TLBIALLEL tlbiOp(EL1, haveSecurity && !scr.ns);
+tlbiOp(tc);
+return;
+}
+ // AArch64 TLB Invalidate All, EL1, Inner Shareable
+ case MISCREG_TLBI_ALLE1IS:
+{
+assert64();
+scr = readMiscReg(MISCREG_SCR);
+
+TLBIALLEL tlbiOp(EL1, haveSecurity && !scr.ns);
+tlbiOp.broadcast(tc);
+return;
+}
case MISCREG_TLBI_VMALLS12E1:
// @todo: handle VMID and stage 2 to enable Virtualization
{
@@ -1759,8 +1777,6 @@
tlbiOp(tc);
return;
}
- // AArch64 TLB Invalidate All, EL1, Inner Shareable
- case MISCREG_TLBI_ALLE1IS:
case MISCREG_TLBI_VMALLS12E1IS:
// @todo: handle VMID and stage 2 to enable Virtualization
{
diff --git a/src/arch/arm/tlb.cc b/src/arch/arm/tlb.cc
index a8e0cb3..56bce27 100644
--- a/src/arch/arm/tlb.cc
+++ b/src/arch/arm/tlb.cc
@@ -300,6 +300,36 @@
}
void
+TLB::flush(const TLBIALLEL &tlbi_op)
+{
+DPRINTF(TLB, "Flushing all TLB entries (%s lookup)\n",
+(tlbi_op.secureLookup ? "secure" : "non-secure"));
+int x = 0;
+TlbEntry *te;
+while (x < size) {
+te = &table[x];
+const bool el_match = te->checkELMatch(
+tlbi_op.targetEL, tlbi_op.inHost);
+if (te->valid && tlbi_op.secureLookup == !te->nstid && el_match) {
+
+DPRINTF(TLB, " - %s\n", te->print());
+te->valid = false;
+stats.flushedEntries++;
+}
+++x;
+}
+
+stats.flushTlb++;
+
+// If there's a second stage TLB (and we're not it)
+// and if we're targeting EL1
+// then flush it as well
+if (!isStage2 && tlbi_op.targetEL == EL1) {
+stage2Tlb->flush(tlbi_op.makeStage2(
[gem5-dev] To understand page table walk for TimingSimpleCPU in X86 architecture
Hi all, I would like to know how page table walking actually works in x86 TimingSImpleCPU, which uses a timing access memory model. Previously, I tried to look at how the page table is walker for AtomicSimpleCPU which uses an atomic access memory model. In function Walker::WalkerState::startWalk(), for timing alone sendpackets() is called while for other access models, there is a loop with stepWalk() called. I see that a four level page table is used in atomic access, but I don't see anything similar to it in timing mode. In short, I can't understand how sendpackets() method does page walking. It would be great if anyone could help me with this. Thank you. -- Regards, Krishnan. ___ 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]: util: Fix an incorrect print statement in git pre-commit hook
Hoa Nguyen has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/36516 )
Change subject: util: Fix an incorrect print statement in git pre-commit
hook
..
util: Fix an incorrect print statement in git pre-commit hook
Change-Id: I13d0a705b6cfab654635380e2adbf36243344a62
Signed-off-by: Hoa Nguyen
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/36516
Reviewed-by: Gabe Black
Reviewed-by: Jason Lowe-Power
Maintainer: Gabe Black
Tested-by: kokoro
---
M util/git-pre-commit.py
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Gabe Black: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/util/git-pre-commit.py b/util/git-pre-commit.py
index 7681b87..ece2467 100755
--- a/util/git-pre-commit.py
+++ b/util/git-pre-commit.py
@@ -114,6 +114,6 @@
"fixes for commit in\n"
"the following files: ", file=sys.stderr)
for f in staged_mismatch:
-print("\t%s".format(f), file=sys.stderr)
+print("\t{}".format(f), file=sys.stderr)
print("Please `git --add' them", file=sys.stderr)
sys.exit(1)
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/36516
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: I13d0a705b6cfab654635380e2adbf36243344a62
Gerrit-Change-Number: 36516
Gerrit-PatchSet: 2
Gerrit-Owner: Hoa Nguyen
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Hoa Nguyen
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]: mem-cache: Undefine compression ratio of perfect compression
Daniel Carvalho has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/36495 )
Change subject: mem-cache: Undefine compression ratio of perfect compression
..
mem-cache: Undefine compression ratio of perfect compression
Commit c0d67b2263aab6a729368373d9cdef9883870241 assumes that the
cache contains a parameter for its compression ratio. This is not
the case upstream, so force the user to provide it instead.
Change-Id: Ic7b4878bede6b0a34e4adfe7e0aa65a0ee48d1f6
Signed-off-by: Daniel R. Carvalho
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/36495
Reviewed-by: Jason Lowe-Power
Reviewed-by: Nikos Nikoleris
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/mem/cache/compressors/Compressors.py
1 file changed, 3 insertions(+), 2 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
Nikos Nikoleris: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/mem/cache/compressors/Compressors.py
b/src/mem/cache/compressors/Compressors.py
index d7dfdab..689a42e 100644
--- a/src/mem/cache/compressors/Compressors.py
+++ b/src/mem/cache/compressors/Compressors.py
@@ -125,8 +125,9 @@
cxx_header = "mem/cache/compressors/perfect.hh"
chunk_size_bits = 64
-max_compression_ratio = Param.Int(Parent.max_compression_ratio,
-"Maximum compression ratio allowed")
+
+max_compression_ratio = Param.Int("Maximum compression ratio allowed")
+
compression_latency = Param.Cycles(1,
"Number of cycles to perform data compression")
decompression_latency = Param.Cycles(1,
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/36495
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: Ic7b4878bede6b0a34e4adfe7e0aa65a0ee48d1f6
Gerrit-Change-Number: 36495
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Carvalho
Gerrit-Reviewer: Daniel Carvalho
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: Nikos Nikoleris
Gerrit-Reviewer: kokoro
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: mem-cache: Make (de)compression latencies params
Daniel Carvalho has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/36497 ) Change subject: mem-cache: Make (de)compression latencies params .. mem-cache: Make (de)compression latencies params Add 4 params to calculate compression and decompression latencies. A pair of params informs how many chunks are parsed per cycle, and the other pair informs how many extra cycles are needed after the chunks are parsed to finish the (de)compression. Change-Id: Ie67b0c298f06a08011f553789e3a9a1d89dd7c4f Signed-off-by: Daniel R. Carvalho Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/36497 Reviewed-by: Nikos Nikoleris Maintainer: Nikos Nikoleris Tested-by: kokoro --- M src/mem/cache/compressors/Compressors.py M src/mem/cache/compressors/base.cc M src/mem/cache/compressors/base.hh M src/mem/cache/compressors/base_delta_impl.hh M src/mem/cache/compressors/cpack.cc M src/mem/cache/compressors/cpack.hh M src/mem/cache/compressors/dictionary_compressor.hh M src/mem/cache/compressors/dictionary_compressor_impl.hh M src/mem/cache/compressors/fpcd.cc M src/mem/cache/compressors/fpcd.hh M src/mem/cache/compressors/multi.cc M src/mem/cache/compressors/perfect.cc 12 files changed, 156 insertions(+), 76 deletions(-) Approvals: Nikos Nikoleris: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/mem/cache/compressors/Compressors.py b/src/mem/cache/compressors/Compressors.py index 689a42e..a1f3706 100644 --- a/src/mem/cache/compressors/Compressors.py +++ b/src/mem/cache/compressors/Compressors.py @@ -41,6 +41,15 @@ "Minimum percentage of the block size, a compressed block must " "achieve to be stored in compressed format") +comp_chunks_per_cycle = Param.Unsigned(1, +"Number of chunks that can be compressed in parallel per cycle.") +comp_extra_latency = Param.Cycles(1, "Number of extra cycles required " +"to finish compression (e.g., due to shifting and packaging).") +decomp_chunks_per_cycle = Param.Unsigned(1, +"Number of chunks that can be decompressed in parallel per cycle.") +decomp_extra_latency = Param.Cycles(1, "Number of extra cycles required " +"to finish decompression (e.g., due to shifting and packaging).") + class BaseDictionaryCompressor(BaseCacheCompressor): type = 'BaseDictionaryCompressor' abstract = True @@ -57,6 +66,12 @@ chunk_size_bits = 64 +# Base-delta compressors achieve 1-cycle latencies +comp_chunks_per_cycle = 8 * Self.block_size / Self.chunk_size_bits +comp_extra_latency = 0 +decomp_chunks_per_cycle = 8 * Self.block_size / Self.chunk_size_bits +decomp_extra_latency = 0 + class Base64Delta16(BaseDictionaryCompressor): type = 'Base64Delta16' cxx_class = 'Compressor::Base64Delta16' @@ -64,6 +79,12 @@ chunk_size_bits = 64 +# Base-delta compressors achieve 1-cycle latencies +comp_chunks_per_cycle = 8 * Self.block_size / Self.chunk_size_bits +comp_extra_latency = 0 +decomp_chunks_per_cycle = 8 * Self.block_size / Self.chunk_size_bits +decomp_extra_latency = 0 + class Base64Delta32(BaseDictionaryCompressor): type = 'Base64Delta32' cxx_class = 'Compressor::Base64Delta32' @@ -71,6 +92,12 @@ chunk_size_bits = 64 +# Base-delta compressors achieve 1-cycle latencies +comp_chunks_per_cycle = 8 * Self.block_size / Self.chunk_size_bits +comp_extra_latency = 0 +decomp_chunks_per_cycle = 8 * Self.block_size / Self.chunk_size_bits +decomp_extra_latency = 0 + class Base32Delta8(BaseDictionaryCompressor): type = 'Base32Delta8' cxx_class = 'Compressor::Base32Delta8' @@ -78,6 +105,12 @@ chunk_size_bits = 32 +# Base-delta compressors achieve 1-cycle latencies +comp_chunks_per_cycle = 8 * Self.block_size / Self.chunk_size_bits +comp_extra_latency = 0 +decomp_chunks_per_cycle = 8 * Self.block_size / Self.chunk_size_bits +decomp_extra_latency = 0 + class Base32Delta16(BaseDictionaryCompressor): type = 'Base32Delta16' cxx_class = 'Compressor::Base32Delta16' @@ -85,6 +118,12 @@ chunk_size_bits = 32 +# Base-delta compressors achieve 1-cycle latencies +comp_chunks_per_cycle = 8 * Self.block_size / Self.chunk_size_bits +comp_extra_latency = 0 +decomp_chunks_per_cycle = 8 * Self.block_size / Self.chunk_size_bits +decomp_extra_latency = 0 + class Base16Delta8(BaseDictionaryCompressor): type = 'Base16Delta8' cxx_class = 'Compressor::Base16Delta8' @@ -92,16 +131,36 @@ chunk_size_bits = 16 +# Base-delta compressors achieve 1-cycle latencies +comp_chunks_per_cycle = 8 * Self.block_size / Self.chunk_size_bits +comp_extra_latency = 0 +decomp_chunks_per_cycle = 8 * Self.block_size / Self.chunk_size_bits +decomp_extra_latency = 0 + class CPack(BaseDictionaryCompressor): t
[gem5-dev] Change in gem5/gem5[develop]: python: Add support for Proxy division
Daniel Carvalho has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/36496 )
Change subject: python: Add support for Proxy division
..
python: Add support for Proxy division
Allow proxies to use python3's division operations. The dividends
and divisors can be either a proxy or a constant.
Change-Id: I96b854355b8f593edfb1ea52a52548b855b05fc0
Signed-off-by: Daniel R. Carvalho
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/36496
Reviewed-by: Andreas Sandberg
Reviewed-by: Jason Lowe-Power
Reviewed-by: Nikos Nikoleris
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/python/m5/proxy.py
1 file changed, 40 insertions(+), 21 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
Andreas Sandberg: Looks good to me, approved
Nikos Nikoleris: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/python/m5/proxy.py b/src/python/m5/proxy.py
index 9d91b84..d15b6f2 100644
--- a/src/python/m5/proxy.py
+++ b/src/python/m5/proxy.py
@@ -55,7 +55,7 @@
def __init__(self, search_self, search_up):
self._search_self = search_self
self._search_up = search_up
-self._multipliers = []
+self._ops = []
def __str__(self):
if self._search_self and not self._search_up:
@@ -72,29 +72,48 @@
"cannot set attribute '%s' on proxy object" % attr)
super(BaseProxy, self).__setattr__(attr, value)
-# support for multiplying proxies by constants or other proxies to
-# other params
-def __mul__(self, other):
-if not (isinstance(other, (int, long, float)) or isproxy(other)):
-raise TypeError(
-"Proxy multiplier must be a constant or a proxy to a
param")
-self._multipliers.append(other)
-return self
+def _gen_op(operation):
+def op(self, operand):
+if not (isinstance(operand, (int, long, float)) or \
+isproxy(operand)):
+raise TypeError(
+"Proxy operand must be a constant or a proxy to a
param")
+self._ops.append((operation, operand))
+return self
+return op
+# Support for multiplying proxies by either constants or other proxies
+__mul__ = _gen_op(lambda operand_a, operand_b : operand_a * operand_b)
__rmul__ = __mul__
-def _mulcheck(self, result, base):
+# Support for dividing proxies by either constants or other proxies
+__truediv__ = _gen_op(lambda operand_a, operand_b :
+operand_a / operand_b)
+__floordiv__ = _gen_op(lambda operand_a, operand_b :
+operand_a // operand_b)
+
+# Support for dividing constants by proxies
+__rtruediv__ = _gen_op(lambda operand_a, operand_b :
+operand_b / operand_a.getValue())
+__rfloordiv__ = _gen_op(lambda operand_a, operand_b :
+operand_b // operand_a.getValue())
+
+# After all the operators and operands have been defined, this function
+# should be called to perform the actual operation
+def _opcheck(self, result, base):
from . import params
-for multiplier in self._multipliers:
-if isproxy(multiplier):
-multiplier = multiplier.unproxy(base)
-# assert that we are multiplying with a compatible
-# param
-if not isinstance(multiplier, params.NumericParamValue):
-raise TypeError(
-"Proxy multiplier must be a numerical param")
-multiplier = multiplier.getValue()
-result = result * multiplier
+for operation, operand in self._ops:
+# Get the operand's value
+if isproxy(operand):
+operand = operand.unproxy(base)
+# assert that we are operating with a compatible param
+if not isinstance(operand, params.NumericParamValue):
+raise TypeError("Proxy operand must be a numerical
param")
+operand = operand.getValue()
+
+# Apply the operation
+result = operation(result, operand)
+
return result
def unproxy(self, base):
@@ -128,7 +147,7 @@
raise RuntimeError("Cycle in unproxy")
result = result.unproxy(obj)
-return self._mulcheck(result, base)
+return self._opcheck(result, base)
def getindex(obj, index):
if index == None:
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/36496
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: I96b854355b8f593edfb1ea52a52548b855b05fc0
Gerrit-Change-Number: 36496
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Carvalho
Gerrit-Reviewer: Andreas Sandber
[gem5-dev] Change in gem5/gem5[develop]: mem-cache: Implement FPC cache compressor
Daniel Carvalho has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/36395 )
Change subject: mem-cache: Implement FPC cache compressor
..
mem-cache: Implement FPC cache compressor
Implementation of Frequent Pattern Compression, proposed
by Alameldeen et al. in "Frequent Pattern Compression: A
Significance-Based Compression Scheme for L2 Caches".
Change-Id: I6dca8ca6b3043b561140bc681dbdbe9f7cef27d7
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/36395
Reviewed-by: Nikos Nikoleris
Reviewed-by: Jason Lowe-Power
Maintainer: Nikos Nikoleris
Tested-by: kokoro
---
M src/mem/cache/compressors/Compressors.py
M src/mem/cache/compressors/SConscript
M src/mem/cache/compressors/dictionary_compressor.hh
A src/mem/cache/compressors/fpc.cc
A src/mem/cache/compressors/fpc.hh
5 files changed, 475 insertions(+), 1 deletion(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Nikos Nikoleris: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/mem/cache/compressors/Compressors.py
b/src/mem/cache/compressors/Compressors.py
index a1f3706..b93a66a 100644
--- a/src/mem/cache/compressors/Compressors.py
+++ b/src/mem/cache/compressors/Compressors.py
@@ -148,6 +148,21 @@
decomp_chunks_per_cycle = 2
decomp_extra_latency = 1
+class FPC(BaseDictionaryCompressor):
+type = 'FPC'
+cxx_class = 'Compressor::FPC'
+cxx_header = "mem/cache/compressors/fpc.hh"
+
+comp_chunks_per_cycle = 8
+comp_extra_latency = 1
+decomp_chunks_per_cycle = 4
+decomp_extra_latency = 1
+
+# Dummy dictionary size, since FPC has no dictionary
+dictionary_size = 1
+
+zero_run_bits = Param.Int(3, "Number of bits of the zero run bit
field")
+
class FPCD(BaseDictionaryCompressor):
type = 'FPCD'
cxx_class = 'Compressor::FPCD'
diff --git a/src/mem/cache/compressors/SConscript
b/src/mem/cache/compressors/SConscript
index d73c2be..f696c04 100644
--- a/src/mem/cache/compressors/SConscript
+++ b/src/mem/cache/compressors/SConscript
@@ -34,6 +34,7 @@
Source('base_dictionary_compressor.cc')
Source('base_delta.cc')
Source('cpack.cc')
+Source('fpc.cc')
Source('fpcd.cc')
Source('multi.cc')
Source('perfect.cc')
diff --git a/src/mem/cache/compressors/dictionary_compressor.hh
b/src/mem/cache/compressors/dictionary_compressor.hh
index e5d67d7..fe70b59 100644
--- a/src/mem/cache/compressors/dictionary_compressor.hh
+++ b/src/mem/cache/compressors/dictionary_compressor.hh
@@ -135,6 +135,8 @@
class RepeatedValuePattern;
template
class DeltaPattern;
+template
+class SignExtendedPattern;
/**
* Create a factory to determine if input matches a pattern. The if
else
@@ -346,7 +348,7 @@
*
* @return The size.
*/
-std::size_t
+virtual std::size_t
getSizeBits() const
{
return numUnmatchedBits + length;
@@ -735,6 +737,55 @@
}
};
+/**
+ * A pattern that checks whether the value is an N bits sign-extended
value,
+ * that is, all the MSB starting from the Nth are equal to the (N-1)th bit.
+ *
+ * Therefore, if N = 8, and T has 16 bits, the values within the ranges
+ * [0x, 0x007F] and [0xFF80, 0x] would match this pattern.
+ *
+ * @tparam N The number of bits in the non-extended original value. It must
+ * fit in a dictionary entry.
+ */
+template
+template
+class DictionaryCompressor::SignExtendedPattern
+: public DictionaryCompressor::Pattern
+{
+ private:
+static_assert((N > 0) & (N <= (sizeof(T) * 8)),
+"The original data's type size must be smaller than the
dictionary's");
+
+/** The non-extended original value. */
+const T bits : N;
+
+ public:
+SignExtendedPattern(const int number,
+const uint64_t code,
+const uint64_t metadata_length,
+const DictionaryEntry bytes,
+const bool allocate = false)
+ : DictionaryCompressor::Pattern(number, code, metadata_length, N,
+-1, allocate),
+bits(fromDictionaryEntry(bytes) & mask(N))
+{
+}
+
+static bool
+isPattern(const DictionaryEntry& bytes,
+const DictionaryEntry& dict_bytes, const int match_location)
+{
+const T data = DictionaryCompressor::fromDictionaryEntry(bytes);
+return data == sext(data & mask(N));
+}
+
+DictionaryEntry
+decompress(const DictionaryEntry dict_bytes) const override
+{
+return toDictionaryEntry(sext(bits));
+}
+};
+
} // namespace Compressor
#endif //__MEM_CACHE_COMPRESSORS_DICTIONARY_COMPRESSOR_HH__
diff --git a/src/mem/cache/compressors/fpc.cc
b/src/mem/cache/compressors/fpc.cc
new file mode 100644
index 000..8398a32
--- /dev/null
+++ b/src/mem/cache/compressors/fpc.cc
@@ -0,0 +1,103 @@
+/*
+ * Copyright (c) 2018-2020 Inria
+ * All rights reserved.
+ *
+ * Redistribution and use in sourc
[gem5-dev] Change in gem5/gem5[develop]: fastmodel: Fix up for the new standardized create() methods.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/36155 )
Change subject: fastmodel: Fix up for the new standardized create() methods.
..
fastmodel: Fix up for the new standardized create() methods.
Change-Id: I2e3610b5cad37b67d32907a2c2568b504d5ed113
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/36155
Reviewed-by: Andreas Sandberg
Maintainer: Andreas Sandberg
Tested-by: kokoro
---
M src/arch/arm/fastmodel/CortexA76/cortex_a76.cc
M src/arch/arm/fastmodel/CortexA76/cortex_a76.hh
M src/arch/arm/fastmodel/CortexR52/cortex_r52.cc
M src/arch/arm/fastmodel/CortexR52/cortex_r52.hh
M src/arch/arm/fastmodel/GIC/gic.cc
5 files changed, 8 insertions(+), 8 deletions(-)
Approvals:
Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/fastmodel/CortexA76/cortex_a76.cc
b/src/arch/arm/fastmodel/CortexA76/cortex_a76.cc
index a9206de..802299b 100644
--- a/src/arch/arm/fastmodel/CortexA76/cortex_a76.cc
+++ b/src/arch/arm/fastmodel/CortexA76/cortex_a76.cc
@@ -98,8 +98,8 @@
return Base::getPort(if_name, idx);
}
-CortexA76Cluster::CortexA76Cluster(Params &p) :
-SimObject(&p), _params(p), cores(p.cores), evs(p.evs)
+CortexA76Cluster::CortexA76Cluster(const Params &p) :
+SimObject(p), _params(p), cores(p.cores), evs(p.evs)
{
for (int i = 0; i < p.cores.size(); i++)
p.cores[i]->setCluster(this, i);
diff --git a/src/arch/arm/fastmodel/CortexA76/cortex_a76.hh
b/src/arch/arm/fastmodel/CortexA76/cortex_a76.hh
index 6088b0a..68ff1a8 100644
--- a/src/arch/arm/fastmodel/CortexA76/cortex_a76.hh
+++ b/src/arch/arm/fastmodel/CortexA76/cortex_a76.hh
@@ -62,8 +62,8 @@
const Params ¶ms() { return _params; }
public:
-CortexA76(Params &p) : Base(&p,
scx::scx_get_iris_connection_interface()),
-_params(p)
+CortexA76(const Params &p) :
+Base(p, scx::scx_get_iris_connection_interface()), _params(p)
{}
void
@@ -110,7 +110,7 @@
CortexA76 *getCore(int num) const { return cores.at(num); }
sc_core::sc_module *getEvs() const { return evs; }
-CortexA76Cluster(Params &p);
+CortexA76Cluster(const Params &p);
const Params ¶ms() { return _params; }
Port &getPort(const std::string &if_name,
diff --git a/src/arch/arm/fastmodel/CortexR52/cortex_r52.cc
b/src/arch/arm/fastmodel/CortexR52/cortex_r52.cc
index 1936e2a..31485d2 100644
--- a/src/arch/arm/fastmodel/CortexR52/cortex_r52.cc
+++ b/src/arch/arm/fastmodel/CortexR52/cortex_r52.cc
@@ -94,7 +94,7 @@
}
CortexR52Cluster::CortexR52Cluster(const Params &p) :
-SimObject(&p), _params(p), cores(p.cores), evs(p.evs)
+SimObject(p), _params(p), cores(p.cores), evs(p.evs)
{
for (int i = 0; i < p.cores.size(); i++)
p.cores[i]->setCluster(this, i);
diff --git a/src/arch/arm/fastmodel/CortexR52/cortex_r52.hh
b/src/arch/arm/fastmodel/CortexR52/cortex_r52.hh
index 9abf194..8332066 100644
--- a/src/arch/arm/fastmodel/CortexR52/cortex_r52.hh
+++ b/src/arch/arm/fastmodel/CortexR52/cortex_r52.hh
@@ -63,7 +63,7 @@
public:
CortexR52(const Params &p) :
-Base(&p, scx::scx_get_iris_connection_interface()), _params(p)
+Base(p, scx::scx_get_iris_connection_interface()), _params(p)
{}
template
diff --git a/src/arch/arm/fastmodel/GIC/gic.cc
b/src/arch/arm/fastmodel/GIC/gic.cc
index 97d0154..934305b 100644
--- a/src/arch/arm/fastmodel/GIC/gic.cc
+++ b/src/arch/arm/fastmodel/GIC/gic.cc
@@ -297,7 +297,7 @@
}
GIC::GIC(const FastModelGICParams ¶ms) :
-BaseGic(¶ms),
+BaseGic(params),
ambaM(params.sc_gic->amba_m, params.name + ".amba_m", -1),
ambaS(params.sc_gic->amba_s, params.name + ".amba_s", -1),
redistributors(params.port_redistributor_connection_count),
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/36155
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: I2e3610b5cad37b67d32907a2c2568b504d5ed113
Gerrit-Change-Number: 36155
Gerrit-PatchSet: 2
Gerrit-Owner: Gabe Black
Gerrit-Reviewer: Andreas Sandberg
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] Re: Changes to SE mode
Hey folks! If someone is going to look at these I can wait, but if not there's no point in just letting it sit there... Gabe On Thu, Oct 22, 2020 at 4:23 PM Gabe Black wrote: > For fans of the classics, you can look up my email with subject "Merging > of FS and SE mode" from 2011, but I suspect that's too old for most people > to have seen, and not completely the same as what I'm trying to do here. > This is the other half of the workload concept which is already applied to > FS mode, ie separating and modularizing the thing you're running from the > thing you're running on. The "hardware" like the CPU, memory, devices, etc, > is the same no matter if you're running SE mode programs, FS operating > systems, an OS from scratch, bare metal, etc. Then on top of that, you can > add a workload object which will load an OS for you like the system objects > used to do and the workload objects do now, or which fakes an OS kernel and > loads and coordinates SE mode processes which is what these new changes are > setting up, or sets up SE mode like hooks for a BIOS to implement to fake > power modes or BIOS interrupt routines for something like DOS, etc, etc. > > A long time ago I managed to break down the barriers between SE and FS > modes to the point where they could be combined into a single build, but > there is still a big FS/SE mode switch which means you have to pick which > one you want, and a decent number of places where global behaviors or > mechanisms are selected with that switch. These changes are a big step > towards not having that switch at all, and for SE mode and FS mode to not > be modes, but to just be handy labels to put on configurations which tend > to fit in certain categories. Other than just making gem5 cleaner and > simpler, this will also enable some configurations which are just not > possible today, like systems which fall outside of the FS or SE dichotomy, > or which combine them and have, for instance, an FS mode server connected > to SE mode clients over a simulated network. > > Gabe > > On Thu, Oct 22, 2020 at 8:06 AM Jason Lowe-Power via gem5-dev < > gem5-dev@gem5.org> wrote: > >> Hi Gabe, >> >> Could you give us some context to the 16 changes that you are going to >> commit without review at the end of the week? Starting with >> https://gem5-review.googlesource.com/c/public/gem5/+/33902/ >> >> I would like to review these, but I don't have any context as to the >> purpose. What problem are they solving and how do they solve the problem? >> >> Sorry that I missed these for two months. You've contributed over 116 >> changes in the past two months. That requires others to review about two >> changes per day. There's bound to be things that fall through the cracks. >> >> Thanks, >> 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 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
