[gem5-dev] Change in gem5/gem5[develop]: util: Allow overriding the magic address in the m5 utility.
Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/27244 ) Change subject: util: Allow overriding the magic address in the m5 utility. .. util: Allow overriding the magic address in the m5 utility. This is useful in situations where the address is hard to know ahead of time, for instance on ARM systems where the address map is hard to predict. The default address is now M5OP_ADDR, or 0 if that's not defined. Change-Id: I3140e05b04365c1a76e52f8c3dc85f472c230ae4 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/27244 Maintainer: Gabe Black Tested-by: kokoro Reviewed-by: Giacomo Travaglini --- M util/m5/src/addr_call_type.c M util/m5/src/addr_call_type.h M util/m5/src/m5.c M util/m5/src/m5_mmap.c M util/m5/src/m5_mmap.h 5 files changed, 57 insertions(+), 8 deletions(-) Approvals: Giacomo Travaglini: Looks good to me, approved Gabe Black: Looks good to me, approved kokoro: Regressions pass diff --git a/util/m5/src/addr_call_type.c b/util/m5/src/addr_call_type.c index cb269dc..0b3d1fc 100644 --- a/util/m5/src/addr_call_type.c +++ b/util/m5/src/addr_call_type.c @@ -28,6 +28,8 @@ #include #include "addr_call_type.h" +#include "args.h" +#include "m5_mmap.h" #define M5OP(name, func) __typeof__(name) M5OP_MERGE_TOKENS(name, _addr); M5OP_FOREACH @@ -42,9 +44,40 @@ int addr_call_type_detect(int *argc, char **argv[]) { -if (*argc > 0 && strcmp((*argv)[0], "--addr") == 0) { +static const char *prefix = "--addr"; +const size_t prefix_len = strlen(prefix); +uint64_t addr_override; + +// If the first argument starts with --addr... +if (*argc > 0 && memcmp((*argv)[0], prefix, prefix_len) == 0) { +char *argv0 = (*argv)[0]; (*argc)--; (*argv)++; + +// If there's more text in this argument... +if (strlen(argv0) != prefix_len) { +// If it doesn't start with '=', it's malformed. +if (argv0[prefix_len] != '=') +return -1; +// Attempt to extract an address after the '='. +char *temp_argv[] = { &argv0[prefix_len + 1] }; +if (!parse_int_args(1, temp_argv, &addr_override, 1)) +return -1; +// If we found an address, use it to override m5op_addr. +m5op_addr = addr_override; +return 1; +} +// If an address override wasn't part of the first argument, check if +// it's the second argument. If not, then there's no override. +if (*argc > 0 && parse_int_args(1, *argv, &addr_override, 1)) { +m5op_addr = addr_override; +(*argc)--; +(*argv)++; +return 1; +} +// If the default address was zero, an override is required. +if (!m5op_addr) +return -1; return 1; } return 0; diff --git a/util/m5/src/addr_call_type.h b/util/m5/src/addr_call_type.h index d1fbfa6..6dcdb5b 100644 --- a/util/m5/src/addr_call_type.h +++ b/util/m5/src/addr_call_type.h @@ -30,6 +30,7 @@ #include "dispatch_table.h" +// Returns 0 if not detected, 1 if detected successfully, and -1 on error. int addr_call_type_detect(int *argc, char **argv[]); DispatchTable *addr_call_type_init(); diff --git a/util/m5/src/m5.c b/util/m5/src/m5.c index 11e7d60..644acd0 100644 --- a/util/m5/src/m5.c +++ b/util/m5/src/m5.c @@ -290,10 +290,16 @@ fprintf(stderr, "\n"); fprintf(stderr, "Call types:\n"); # if ENABLE_CT_addr -fprintf(stderr, "--addr%s\n", DEFAULT_CT_addr ? " (default)" : ""); +fprintf(stderr, "--addr %s%s\n", +# if defined(M5OP_ADDR) +"[address override]", +# else +"", +# endif +DEFAULT_CT_addr ? " (default)" : ""); fprintf(stderr, "Use the address based invocation method.\n"); # if defined(M5OP_ADDR) -fprintf(stderr, "The address is %#"PRIx64".\n", +fprintf(stderr, "The default address is %#"PRIx64".\n", (uint64_t)M5OP_ADDR); # endif # endif @@ -331,8 +337,12 @@ } # endif # if ENABLE_CT_addr -if (!dt && addr_call_type_detect(&argc, &argv)) { -dt = addr_call_type_init(); +if (!dt) { +int detect = addr_call_type_detect(&argc, &argv); +if (detect < 0) +usage(); +if (detect > 0) +dt = addr_call_type_init(); } # endif # if ENABLE_CT_semi diff --git a/util/m5/src/m5_mmap.c b/util/m5/src/m5_mmap.c index 79de59b..4a5aa0f 100644 --- a/util/m5/src/m5_mmap.c +++ b/util/m5/src/m5_mmap.c @@ -49,10 +49,14 @@ void *m5_mem = NULL; +#ifndef M5OP_ADDR +#define M5OP_ADDR 0 +#endif +uint64_t m5op_addr = M5OP_ADDR; + void map_m5_mem() { -#ifdef M5OP_ADDR int fd; fd = open("/dev/mem", O_RDWR | O_SYNC); @@ -62,10 +66,9 @@ } m5_mem = mmap(NULL, 0x1, PROT_READ | PROT_WRITE, MAP_SHA
[gem5-dev] Change in gem5/gem5[develop]: util: Add a semihosting implementation to the aarch64 m5 utility.
Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/27245 ) Change subject: util: Add a semihosting implementation to the aarch64 m5 utility. .. util: Add a semihosting implementation to the aarch64 m5 utility. This will allow it to work on CPUs that only support semihosting like ARM's fastmodels. Change-Id: I74e536d79d0f77b864e1e4b9d73e265b6d0b1fcb Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/27245 Reviewed-by: Jason Lowe-Power Maintainer: Jason Lowe-Power Tested-by: kokoro --- M util/m5/src/aarch64/SConsopts A util/m5/src/aarch64/m5op_semi.S 2 files changed, 92 insertions(+), 0 deletions(-) Approvals: Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/util/m5/src/aarch64/SConsopts b/util/m5/src/aarch64/SConsopts index 343247c..6032aea 100644 --- a/util/m5/src/aarch64/SConsopts +++ b/util/m5/src/aarch64/SConsopts @@ -30,3 +30,4 @@ env['CALL_TYPE']['inst'].impl('m5op.S', default=True) env['CALL_TYPE']['addr'].impl('m5op_addr.S') +env['CALL_TYPE']['semi'].impl('m5op_semi.S') diff --git a/util/m5/src/aarch64/m5op_semi.S b/util/m5/src/aarch64/m5op_semi.S new file mode 100644 index 000..744b1300 --- /dev/null +++ b/util/m5/src/aarch64/m5op_semi.S @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2010-2013, 2016-2017 ARM Limited + * All rights reserved + * + * The license below extends only to copyright in the software and shall + * not be construed as granting a license to any other intellectual + * property including but not limited to intellectual property relating + * to a hardware implementation of the functionality of the software + * licensed hereunder. You may use the software subject to the license + * terms below provided that you ensure that this notice is replicated + * unmodified and in its entirety in all distributions of the software, + * modified or unmodified, in source code or in binary form. + * + * Copyright (c) 2003-2006 The Regents of The University of Michigan + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer; + * redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution; + * neither the name of the copyright holders nor the names of its + * contributors may be used to endorse or promote products derived from + * this 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 + +.macro m5op_func, name, func +.globl \name +\name: +// Put the m5 op number in x16. +mov x16, #(\func << 8) +// Branch into the common handler for the rest. +b 1f +.endm + +.text +#define M5OP(name, func) m5op_func M5OP_MERGE_TOKENS(name, _semi), func; +M5OP_FOREACH +#undef M5OP + +1: +// Get the address of the argument block. +ldr x17, =m5_semi_argument_block +// Store the m5 op number in the first slot. +str x16, [ x17 ], #8 +// Store all 8 possible arguments in the subsequent slots. We don't +// know how many we need, so just store them all. +str x0, [ x17 ], #8 +str x1, [ x17 ], #8 +str x2, [ x17 ], #8 +str x3, [ x17 ], #8 +str x4, [ x17 ], #8 +str x5, [ x17 ], #8 +str x6, [ x17 ], #8 +str x7, [ x17 ], #8 +// Set x0 to the m5 op semi-hosting call number. +mov x0, #0x100 +// Set x1 to the address of the argument blob. +ldr x1, =m5_semi_argument_block +// Trigger the semihosting call with the gem5 specific immediate. +hlt #0x5d57 +ret + +.data +.globl m5_semi_argument_block +m5_semi_argument_block: +.quad 0 // function +.quad 0 // argument 0
[gem5-dev] Change in gem5/gem5[develop]: arm: Teach gem5 to recognize the gem5 semihosting immediate values.
Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/27246 ) Change subject: arm: Teach gem5 to recognize the gem5 semihosting immediate values. .. arm: Teach gem5 to recognize the gem5 semihosting immediate values. These give access to the gem5 extension calls, currently only the pseudo ops. Change-Id: I60ece82f1f084791971a2de0b54be2f0d9da243e Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/27246 Reviewed-by: Giacomo Travaglini Maintainer: Giacomo Travaglini Tested-by: kokoro Tested-by: Gem5 Cloud Project GCB service account <345032938...@cloudbuild.gserviceaccount.com> --- M src/arch/arm/isa/includes.isa M src/arch/arm/isa/insts/misc.isa M src/arch/arm/isa/insts/misc64.isa M src/arch/arm/semihosting.hh 4 files changed, 23 insertions(+), 4 deletions(-) Approvals: Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass Gem5 Cloud Project GCB service account: Regressions pass diff --git a/src/arch/arm/isa/includes.isa b/src/arch/arm/isa/includes.isa index 9cdc1f9..14d1c55 100644 --- a/src/arch/arm/isa/includes.isa +++ b/src/arch/arm/isa/includes.isa @@ -98,6 +98,7 @@ #include "arch/arm/isa.hh" #include "arch/arm/isa_traits.hh" #include "arch/arm/pauth_helpers.hh" +#include "arch/arm/semihosting.hh" #include "arch/arm/utility.hh" #include "arch/generic/memhelpers.hh" #include "base/condcodes.hh" diff --git a/src/arch/arm/isa/insts/misc.isa b/src/arch/arm/isa/insts/misc.isa index e8935b8..b2f4591 100644 --- a/src/arch/arm/isa/insts/misc.isa +++ b/src/arch/arm/isa/insts/misc.isa @@ -40,10 +40,14 @@ svcCode = ''' ThreadContext *tc = xc->tcBase(); -const auto semihost_imm = Thumb? 0xAB : 0x123456; - -if (ArmSystem::haveSemihosting(tc) && imm == semihost_imm) { +bool have_semi = ArmSystem::haveSemihosting(tc); +if (have_semi && Thumb && imm == ArmSemihosting::T32Imm) { +// Enable gem5 extensions since we can't distinguish in thumb. +ArmSystem::callSemihosting32(tc, true); +} else if (have_semi && imm == ArmSemihosting::A32Imm) { ArmSystem::callSemihosting32(tc); +} else if (have_semi && imm == ArmSemihosting::Gem5Imm) { +ArmSystem::callSemihosting32(tc, true); } else { fault = std::make_shared(machInst, imm); } diff --git a/src/arch/arm/isa/insts/misc64.isa b/src/arch/arm/isa/insts/misc64.isa index e2cfb41..656a234 100644 --- a/src/arch/arm/isa/insts/misc64.isa +++ b/src/arch/arm/isa/insts/misc64.isa @@ -185,8 +185,11 @@ hltCode = ''' ThreadContext *tc = xc->tcBase(); -if (ArmSystem::haveSemihosting(tc) && imm == 0xF000) { +bool have_semi = ArmSystem::haveSemihosting(tc); +if (imm == ArmSemihosting::A64Imm && have_semi) { ArmSystem::callSemihosting64(tc); +} else if (imm == ArmSemihosting::Gem5Imm && have_semi) { +ArmSystem::callSemihosting64(tc, true); } else { // HLT instructions aren't implemented, so treat them as undefined // instructions. diff --git a/src/arch/arm/semihosting.hh b/src/arch/arm/semihosting.hh index 83d41fd..e9dc984 100644 --- a/src/arch/arm/semihosting.hh +++ b/src/arch/arm/semihosting.hh @@ -73,6 +73,17 @@ { public: +enum { +// Standard ARM immediate values which trigger semihosting. +T32Imm = 0xAB, +A32Imm = 0x123456, +A64Imm = 0xF000, + +// The immediate value which enables gem5 semihosting calls. Use the +// standard value for thumb. +Gem5Imm = 0x5D57 +}; + static PortProxy &portProxy(ThreadContext *tc); struct AbiBase -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/27246 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: I60ece82f1f084791971a2de0b54be2f0d9da243e Gerrit-Change-Number: 27246 Gerrit-PatchSet: 19 Gerrit-Owner: Gabe Black Gerrit-Reviewer: Bobby R. Bruce Gerrit-Reviewer: Ciro Santilli Gerrit-Reviewer: Earl Ou Gerrit-Reviewer: Gabe Black Gerrit-Reviewer: Gem5 Cloud Project GCB service account <345032938...@cloudbuild.gserviceaccount.com> Gerrit-Reviewer: Giacomo Travaglini Gerrit-Reviewer: Jason Lowe-Power Gerrit-Reviewer: Yu-hsin Wang Gerrit-Reviewer: kokoro Gerrit-MessageType: merged ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: scons: Add MARSHAL_XXFLAGS_EXTRA for the marshal object
Giacomo Travaglini has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/30016 ) Change subject: scons: Add MARSHAL_XXFLAGS_EXTRA for the marshal object .. scons: Add MARSHAL_XXFLAGS_EXTRA for the marshal object We already provide to the user the CCFLAGS_EXTRA, LDFLAGS_EXTRA variables to pass flags to scons when compiling/linking gem5. Those variables are not passed to the marshal object. We add an extra pair: MARSHAL_CCFLAGS_EXTRA, MARSHAL_LDFLAGS_EXTRA to add flag injection capabilities to the marshal object. The patch is also renaming base_py_env to marshal_env. This happens for 2 reasons: 1) At the moment the marshal compilation is the only task making use of the base python environment. 2) Consistency with the EXTRA variable names added with this patch. I could have named them as BASE_XXFLAGS_EXTRA, but it seems too much generic and users might be confused by that, as they might think the BASE_XXFLAGS_EXTRA is a subset of the XXFLAGS_EXTRA so that setting it will affect gem5 compilation as well. Change-Id: I3e420caa897059455ff8f35462db2b38da050e93 Signed-off-by: Giacomo Travaglini Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/30016 Reviewed-by: Jason Lowe-Power Reviewed-by: Nikos Nikoleris Reviewed-by: Andreas Sandberg Maintainer: Andreas Sandberg Tested-by: kokoro --- M SConstruct M src/SConscript 2 files changed, 8 insertions(+), 4 deletions(-) Approvals: Jason Lowe-Power: Looks good to me, but someone else must approve Andreas Sandberg: Looks good to me, approved; Looks good to me, approved Nikos Nikoleris: Looks good to me, approved kokoro: Regressions pass diff --git a/SConstruct b/SConstruct index 4bc3d0e..b327a8b 100755 --- a/SConstruct +++ b/SConstruct @@ -276,6 +276,8 @@ ('CXX', 'C++ compiler', environ.get('CXX', main['CXX'])), ('CCFLAGS_EXTRA', 'Extra C and C++ compiler flags', ''), ('LDFLAGS_EXTRA', 'Extra linker flags', ''), +('MARSHAL_CCFLAGS_EXTRA', 'Extra C and C++ marshal compiler flags', ''), +('MARSHAL_LDFLAGS_EXTRA', 'Extra marshal linker flags', ''), ('PYTHON_CONFIG', 'Python config binary to use', [ 'python2.7-config', 'python-config', 'python3-config' ]), ('PROTOC', 'protoc tool', environ.get('PROTOC', 'protoc')), @@ -734,7 +736,9 @@ main.Prepend(CPPPATH=Dir('ext/pybind11/include/')) # Bare minimum environment that only includes python -base_py_env = main.Clone() +marshal_env = main.Clone() +marshal_env.Append(CCFLAGS='$MARSHAL_CCFLAGS_EXTRA') +marshal_env.Append(LINKFLAGS='$MARSHAL_LDFLAGS_EXTRA') # On Solaris you need to use libsocket for socket ops if not conf.CheckLibWithHeader(None, 'sys/socket.h', 'C++', 'accept(0,0,0);'): @@ -1285,7 +1289,7 @@ # to the configured variables. It returns a list of environments, # one for each variant build (debug, opt, etc.) SConscript('src/SConscript', variant_dir=variant_path, - exports=['env', 'base_py_env']) + exports=['env', 'marshal_env']) # base help text Help(''' diff --git a/src/SConscript b/src/SConscript index 7582510..0b3127f 100644 --- a/src/SConscript +++ b/src/SConscript @@ -1140,7 +1140,7 @@ # Build a small helper that marshals the Python code using the same # version of Python as gem5. This is in an unorthodox location to # avoid building it for every variant. -py_marshal = base_py_env.Program('marshal', 'python/marshal.cc')[0] +py_marshal = marshal_env.Program('marshal', 'python/marshal.cc')[0] # Embed python files. All .py files that have been indicated by a # PySource() call in a SConscript need to be embedded into the M5 @@ -1196,7 +1196,7 @@ code.write(str(target[0])) for source in PySource.all: -base_py_env.Command(source.cpp, [ py_marshal, source.tnode ], +marshal_env.Command(source.cpp, [ py_marshal, source.tnode ], MakeAction(embedPyFile, Transform("EMBED PY"))) Source(source.cpp, tags=source.tags, add_tags='python') -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/30016 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: I3e420caa897059455ff8f35462db2b38da050e93 Gerrit-Change-Number: 30016 Gerrit-PatchSet: 3 Gerrit-Owner: Giacomo Travaglini Gerrit-Reviewer: Andreas Sandberg Gerrit-Reviewer: Bobby R. Bruce Gerrit-Reviewer: Ciro Santilli Gerrit-Reviewer: Gabe Black Gerrit-Reviewer: Giacomo Travaglini Gerrit-Reviewer: Jason Lowe-Power 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]: cpu: Use new InstRecord faulting flag in cpu models
Giacomo Travaglini has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/30135 ) Change subject: cpu: Use new InstRecord faulting flag in cpu models .. cpu: Use new InstRecord faulting flag in cpu models This patch sets the faulting flag in atomic, timing, minor and o3 CPU models. It also fixes the minor/timing CPU models which were not respecting the ExecFaulting flag. This is now checked before calling dump() on the tracing object, to bring it in line with the other CPU models. Change-Id: I9c7b64cc5605596eb7fcf25fdecaeac5c4b5e3d7 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/30135 Reviewed-by: Jason Lowe-Power Maintainer: Jason Lowe-Power Tested-by: kokoro --- M src/cpu/minor/execute.cc M src/cpu/o3/commit_impl.hh M src/cpu/simple/atomic.cc M src/cpu/simple/base.cc M src/cpu/simple/base.hh M src/cpu/simple/timing.cc 6 files changed, 45 insertions(+), 23 deletions(-) Approvals: Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/cpu/minor/execute.cc b/src/cpu/minor/execute.cc index d311d14..3c94531 100644 --- a/src/cpu/minor/execute.cc +++ b/src/cpu/minor/execute.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2014,2018-2019 ARM Limited + * Copyright (c) 2013-2014,2018-2020 ARM Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -48,6 +48,7 @@ #include "debug/Activity.hh" #include "debug/Branch.hh" #include "debug/Drain.hh" +#include "debug/ExecFaulting.hh" #include "debug/MinorExecute.hh" #include "debug/MinorInterrupt.hh" #include "debug/MinorMem.hh" @@ -978,6 +979,15 @@ committed = true; if (fault != NoFault) { +if (inst->traceData) { +if (DTRACE(ExecFaulting)) { +inst->traceData->setFaulting(true); +} else { +delete inst->traceData; +inst->traceData = NULL; +} +} + DPRINTF(MinorExecute, "Fault in execute of inst: %s fault: %s\n", *inst, fault->name()); fault->invoke(thread, inst->staticInst); diff --git a/src/cpu/o3/commit_impl.hh b/src/cpu/o3/commit_impl.hh index 667f42b..4f467e9 100644 --- a/src/cpu/o3/commit_impl.hh +++ b/src/cpu/o3/commit_impl.hh @@ -1,6 +1,6 @@ /* * Copyright 2014 Google, Inc. - * Copyright (c) 2010-2014, 2017 ARM Limited + * Copyright (c) 2010-2014, 2017, 2020 ARM Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -1260,6 +1260,7 @@ tid, head_inst->seqNum); if (head_inst->traceData) { if (DTRACE(ExecFaulting)) { +head_inst->traceData->setFaulting(true); head_inst->traceData->setFetchSeq(head_inst->seqNum); head_inst->traceData->setCPSeq(thread[tid]->numOp); head_inst->traceData->dump(); diff --git a/src/cpu/simple/atomic.cc b/src/cpu/simple/atomic.cc index 4671402..c57fe14 100644 --- a/src/cpu/simple/atomic.cc +++ b/src/cpu/simple/atomic.cc @@ -1,6 +1,6 @@ /* * Copyright 2014 Google, Inc. - * Copyright (c) 2012-2013,2015,2017-2019 ARM Limited + * Copyright (c) 2012-2013,2015,2017-2020 ARM Limited * All rights reserved. * * The license below extends only to copyright in the software and shall @@ -713,10 +713,8 @@ if (fault == NoFault) { countInst(); ppCommit->notify(std::make_pair(thread, curStaticInst)); -} -else if (traceData && !DTRACE(ExecFaulting)) { -delete traceData; -traceData = NULL; +} else if (traceData) { +traceFault(); } if (fault != NoFault && diff --git a/src/cpu/simple/base.cc b/src/cpu/simple/base.cc index c6d5761..1dac921 100644 --- a/src/cpu/simple/base.cc +++ b/src/cpu/simple/base.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2012, 2015, 2017, 2018 ARM Limited + * Copyright (c) 2010-2012, 2015, 2017, 2018, 2020 ARM Limited * Copyright (c) 2013 Advanced Micro Devices, Inc. * All rights reserved * @@ -64,6 +64,7 @@ #include "cpu/static_inst.hh" #include "cpu/thread_context.hh" #include "debug/Decode.hh" +#include "debug/ExecFaulting.hh" #include "debug/Fetch.hh" #include "debug/Quiesce.hh" #include "mem/packet.hh" @@ -433,6 +434,17 @@ } void +BaseSimpleCPU::traceFault() +{ +if (DTRACE(ExecFaulting)) { +traceData->setFaulting(true); +} else { +delete traceData; +traceData = NULL; +} +} + +void BaseSimpleCPU::checkForInterrupts() { SimpleExecContext&t_info = *threadInfo[curThread]; diff --git a/src/cpu/simple/base.hh b/src/cpu/simple/base.hh index 323850a..9f5bf66 100644 --- a/src/c
[gem5-dev] Change in gem5/gem5[develop]: sim: Add faulting flag to instruction tracing interface
Giacomo Travaglini has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/30134 ) Change subject: sim: Add faulting flag to instruction tracing interface .. sim: Add faulting flag to instruction tracing interface This patch adds a faulting flag to InstRecord. This allows tracers to identify that the traced instruction has faulted, when ExecFaulting is enabled. It can be set with InstRecord::setFaulting() and read with Instrecord::getFaulting(). Change-Id: I390392d59de930533eab101e96dc4d3c76500748 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/30134 Reviewed-by: Jason Lowe-Power Maintainer: Jason Lowe-Power Tested-by: kokoro --- M src/sim/insttracer.hh 1 file changed, 13 insertions(+), 2 deletions(-) Approvals: Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/sim/insttracer.hh b/src/sim/insttracer.hh index 2e9806d..284e04a 100644 --- a/src/sim/insttracer.hh +++ b/src/sim/insttracer.hh @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2017 ARM Limited + * Copyright (c) 2014, 2017, 2020 ARM Limited * All rights reserved * * The license below extends only to copyright in the software and shall @@ -143,6 +143,12 @@ */ bool predicate; +/** + * Did the execution of this instruction fault? (requires ExecFaulting + * to be enabled) + */ +bool faulting; + public: InstRecord(Tick _when, ThreadContext *_thread, const StaticInstPtr _staticInst, @@ -151,7 +157,8 @@ : when(_when), thread(_thread), staticInst(_staticInst), pc(_pc), macroStaticInst(_macroStaticInst), addr(0), size(0), flags(0), fetch_seq(0), cp_seq(0), data_status(DataInvalid), mem_valid(false), -fetch_seq_valid(false), cp_seq_valid(false), predicate(true) +fetch_seq_valid(false), cp_seq_valid(false), predicate(true), +faulting(false) { } virtual ~InstRecord() @@ -218,6 +225,8 @@ void setPredicate(bool val) { predicate = val; } +void setFaulting(bool val) { faulting = val; } + virtual void dump() = 0; public: @@ -241,6 +250,8 @@ InstSeqNum getCpSeq() const { return cp_seq; } bool getCpSeqValid() const { return cp_seq_valid; } + +bool getFaulting() const { return faulting; } }; class InstTracer : public SimObject -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/30134 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: I390392d59de930533eab101e96dc4d3c76500748 Gerrit-Change-Number: 30134 Gerrit-PatchSet: 4 Gerrit-Owner: Giacomo Travaglini Gerrit-Reviewer: Anthony Gutierrez 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-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: Fix build errors with gcc 10.x
Sandipan Das has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/30474 ) Change subject: base: Fix build errors with gcc 10.x .. base: Fix build errors with gcc 10.x This fixes conditions that perform a redundant check to see if an unsigned value is greater than or equal to zero. With gcc 10.x, this generates the following error because of implicit usage of the "-Werror=type-limits" flag. "comparison of unsigned expression in '>= 0' is always true" Change-Id: Ib1a88035ef5fba410d18de0adf614db4bc634faf Signed-off-by: Sandipan Das Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/30474 Reviewed-by: Jason Lowe-Power Maintainer: Jason Lowe-Power Tested-by: kokoro --- M src/base/statistics.hh 1 file changed, 4 insertions(+), 4 deletions(-) Approvals: Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/base/statistics.hh b/src/base/statistics.hh index 8f665fe..ee541fb 100644 --- a/src/base/statistics.hh +++ b/src/base/statistics.hh @@ -1161,7 +1161,7 @@ Proxy operator[](off_type index) { -assert (index >= 0 && index < size()); +assert (index < size()); return Proxy(this->self(), index); } }; @@ -1235,7 +1235,7 @@ ScalarProxy operator[](off_type index) { -assert (index >= 0 && index < size()); +assert (index < size()); return ScalarProxy(stat, offset + index); } @@ -1311,7 +1311,7 @@ operator[](off_type index) { off_type offset = index * y; -assert (index >= 0 && offset + y <= size()); +assert (offset + y <= size()); return Proxy(this->self(), offset, y); } @@ -1995,7 +1995,7 @@ Proxy operator[](off_type index) { -assert(index >= 0 && index < size()); +assert(index < size()); return Proxy(this->self(), index); } -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/30474 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: Ib1a88035ef5fba410d18de0adf614db4bc634faf Gerrit-Change-Number: 30474 Gerrit-PatchSet: 3 Gerrit-Owner: Sandipan Das Gerrit-Reviewer: Bobby R. Bruce Gerrit-Reviewer: Jason Lowe-Power Gerrit-Reviewer: Sandipan Das Gerrit-Reviewer: kokoro Gerrit-CC: Gabe Black Gerrit-MessageType: merged ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: arch-gcn3: Updating implementation of atomics
Anthony Gutierrez has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/29926 ) Change subject: arch-gcn3: Updating implementation of atomics .. arch-gcn3: Updating implementation of atomics This changeset is moving the access of the data operand from initiateAcc to the execute method of atomic instructions. Change-Id: I1debae302f0b13f79ed2b7a9ed2f6b07fcec5128 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/29926 Reviewed-by: Anthony Gutierrez Maintainer: Anthony Gutierrez Tested-by: kokoro --- M src/arch/gcn3/insts/instructions.cc 1 file changed, 45 insertions(+), 52 deletions(-) Approvals: Anthony Gutierrez: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/arch/gcn3/insts/instructions.cc b/src/arch/gcn3/insts/instructions.cc index 26af241..32719ad 100644 --- a/src/arch/gcn3/insts/instructions.cc +++ b/src/arch/gcn3/insts/instructions.cc @@ -39261,11 +39261,24 @@ gpuDynInst->latency.set(gpuDynInst->computeUnit()->clockPeriod()); ConstVecOperandU64 addr(gpuDynInst, extData.ADDR); +ConstVecOperandU32 data(gpuDynInst, extData.DATA); +ConstVecOperandU32 cmp(gpuDynInst, extData.DATA + 1); addr.read(); +data.read(); +cmp.read(); calcAddr(gpuDynInst, addr); +for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) { +if (gpuDynInst->exec_mask[lane]) { +(reinterpret_cast(gpuDynInst->x_data))[lane] += data[lane]; +(reinterpret_cast(gpuDynInst->a_data))[lane] += cmp[lane]; +} +} + if (gpuDynInst->executedAs() == Enums::SC_GLOBAL || gpuDynInst->executedAs() == Enums::SC_PRIVATE) { /** @@ -39293,21 +39306,6 @@ void Inst_FLAT__FLAT_ATOMIC_CMPSWAP::initiateAcc(GPUDynInstPtr gpuDynInst) { -ConstVecOperandU32 data(gpuDynInst, extData.DATA); -ConstVecOperandU32 cmp(gpuDynInst, extData.DATA + 1); - -data.read(); -cmp.read(); - -for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) { -if (gpuDynInst->exec_mask[lane]) { -(reinterpret_cast(gpuDynInst->x_data))[lane] -= data[lane]; -(reinterpret_cast(gpuDynInst->a_data))[lane] -= cmp[lane]; -} -} - initAtomicAccess(gpuDynInst); } // initiateAcc @@ -39364,11 +39362,20 @@ gpuDynInst->latency.set(gpuDynInst->computeUnit()->clockPeriod()); ConstVecOperandU64 addr(gpuDynInst, extData.ADDR); +ConstVecOperandU32 data(gpuDynInst, extData.DATA); addr.read(); +data.read(); calcAddr(gpuDynInst, addr); +for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) { +if (gpuDynInst->exec_mask[lane]) { +(reinterpret_cast(gpuDynInst->a_data))[lane] += data[lane]; +} +} + if (gpuDynInst->executedAs() == Enums::SC_GLOBAL) { gpuDynInst->computeUnit()->globalMemoryPipe. issueRequest(gpuDynInst); @@ -39387,17 +39394,6 @@ void Inst_FLAT__FLAT_ATOMIC_ADD::initiateAcc(GPUDynInstPtr gpuDynInst) { -ConstVecOperandU32 data(gpuDynInst, extData.DATA); - -data.read(); - -for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) { -if (gpuDynInst->exec_mask[lane]) { -(reinterpret_cast(gpuDynInst->a_data))[lane] -= data[lane]; -} -} - initAtomicAccess(gpuDynInst); } // initiateAcc @@ -39733,11 +39729,24 @@ gpuDynInst->latency.set(gpuDynInst->computeUnit()->clockPeriod()); ConstVecOperandU64 addr(gpuDynInst, extData.ADDR); +ConstVecOperandU64 data(gpuDynInst, extData.DATA); +ConstVecOperandU64 cmp(gpuDynInst, extData.DATA + 2); addr.read(); +data.read(); +cmp.read(); calcAddr(gpuDynInst, addr); +for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) { +if (gpuDynInst->exec_mask[lane]) { +(reinterpret_cast(gpuDynInst->x_data))[lane] += data[lane]; +(reinterpret_cast(gpuDynInst->a_data))[lane] += cmp[lane]; +} +} + if (gpuDynInst->executedAs() == Enums::SC_GLOBAL || gpuDynInst->executedAs() == Enums::SC_PRIVATE) { /** @@ -39765,21 +39774,6 @@ void Inst_FLAT__FLAT_ATOMIC_CMPSWAP_X2::initiateAcc(GPUDynInstPtr gpuDynInst) { -ConstVecOperandU64 data(gpuDynInst, extData.DATA); -ConstVecOperandU64 cmp(gpuDynInst, extData.DATA + 2); - -data.read(); -cmp.read(); - -for (int lane = 0; lane < NumVecElemPer
[gem5-dev] Change in gem5/gem5[develop]: gpu-compute: Remove unused function hostWakeUp from shader
Anthony Gutierrez has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/29929 ) Change subject: gpu-compute: Remove unused function hostWakeUp from shader .. gpu-compute: Remove unused function hostWakeUp from shader Change-Id: Ib4415a7c5918da03bbd16fe9adb4dd593dcaa95c Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/29929 Reviewed-by: Anthony Gutierrez Maintainer: Anthony Gutierrez Tested-by: kokoro --- M src/gpu-compute/shader.cc M src/gpu-compute/shader.hh 2 files changed, 0 insertions(+), 14 deletions(-) Approvals: Anthony Gutierrez: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/gpu-compute/shader.cc b/src/gpu-compute/shader.cc index aa7a6dd..f5e9444 100644 --- a/src/gpu-compute/shader.cc +++ b/src/gpu-compute/shader.cc @@ -153,19 +153,6 @@ assert(gpuTc); } -void -Shader::hostWakeUp(BaseCPU *cpu) { -if (cpuPointer == cpu) { -if (gpuTc->status() == ThreadContext::Suspended) -cpu->activateContext(gpuTc->threadId()); -} else { -//Make sure both dispatcher and shader are trying to -//wakeup same host. Hack here to enable kernel launch -//from multiple CPUs -panic("Dispatcher wants to wakeup a different host"); -} -} - Shader* ShaderParams::create() { diff --git a/src/gpu-compute/shader.hh b/src/gpu-compute/shader.hh index eeaf343..238f6e0 100644 --- a/src/gpu-compute/shader.hh +++ b/src/gpu-compute/shader.hh @@ -301,7 +301,6 @@ Addr mmap(int length); void functionalTLBAccess(PacketPtr pkt, int cu_id, BaseTLB::Mode mode); void updateContext(int cid); -void hostWakeUp(BaseCPU *cpu); void notifyCuSleep(); }; -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/29929 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: Ib4415a7c5918da03bbd16fe9adb4dd593dcaa95c Gerrit-Change-Number: 29929 Gerrit-PatchSet: 6 Gerrit-Owner: Anthony Gutierrez Gerrit-Reviewer: Anthony Gutierrez Gerrit-Reviewer: Tony Gutierrez Gerrit-Reviewer: Tuan Ta Gerrit-Reviewer: kokoro Gerrit-MessageType: merged ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: dev: add support for HSA's barrier bit kernel synchronization
Anthony Gutierrez has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/29925 ) Change subject: dev: add support for HSA's barrier bit kernel synchronization .. dev: add support for HSA's barrier bit kernel synchronization This commit adds support for the HSA's barrier bit version of synchronization. This method of synchronization is used for all HIP benchmarks, and thus is necessary to ensure that multiple kernels from the same queue are synchronizing properly. Change-Id: I64f2d311a3970b71194e0555e2b932800df65e98 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/29925 Reviewed-by: Anthony Gutierrez Reviewed-by: Matt Sinclair Maintainer: Anthony Gutierrez Tested-by: kokoro --- M src/dev/hsa/hsa_packet_processor.cc M src/dev/hsa/hsa_packet_processor.hh 2 files changed, 39 insertions(+), 3 deletions(-) Approvals: Anthony Gutierrez: Looks good to me, approved; Looks good to me, approved Matt Sinclair: Looks good to me, approved kokoro: Regressions pass diff --git a/src/dev/hsa/hsa_packet_processor.cc b/src/dev/hsa/hsa_packet_processor.cc index f9880e4..4143019 100644 --- a/src/dev/hsa/hsa_packet_processor.cc +++ b/src/dev/hsa/hsa_packet_processor.cc @@ -60,6 +60,11 @@ #define PKT_TYPE(PKT) ((hsa_packet_type_t)(((PKT->header) >> \ HSA_PACKET_HEADER_TYPE) & (HSA_PACKET_HEADER_WIDTH_TYPE - 1))) +// checks if the barrier bit is set in the header -- shift the barrier bit +// to LSB, then bitwise "and" to mask off all other bits +#define IS_BARRIER(PKT) ((hsa_packet_header_t)(((PKT->header) >> \ +HSA_PACKET_HEADER_BARRIER) & HSA_PACKET_HEADER_WIDTH_BARRIER)) + HSAPP_EVENT_DESCRIPTION_GENERATOR(UpdateReadDispIdDmaEvent) HSAPP_EVENT_DESCRIPTION_GENERATOR(CmdQueueCmdDmaEvent) HSAPP_EVENT_DESCRIPTION_GENERATOR(QueueProcessEvent) @@ -280,7 +285,7 @@ HSAPacketProcessor::schedAQLProcessing(uint32_t rl_idx) { RQLEntry *queue = regdQList[rl_idx]; -if (!queue->aqlProcessEvent.scheduled()) { +if (!queue->aqlProcessEvent.scheduled() && !queue->getBarrierBit()) { Tick processingTick = curTick() + pktProcessDelay; schedule(queue->aqlProcessEvent, processingTick); DPRINTF(HSAPacketProcessor, "AQL processing scheduled at tick: %d\n", @@ -316,6 +321,16 @@ // Submit packet to HSA device (dispatcher) hsa_device->submitDispatchPkt((void *)disp_pkt, rl_idx, host_pkt_addr); is_submitted = true; +/* + If this packet is using the "barrier bit" to enforce ordering with + subsequent kernels, set the bit for this queue now, after + dispatching. +*/ +if (IS_BARRIER(disp_pkt)) { +DPRINTF(HSAPacketProcessor, "%s: setting barrier bit for active" \ +" list ID = %d\n", __FUNCTION__, rl_idx); +regdQList[rl_idx]->setBarrierBit(true); +} } else if (pkt_type == HSA_PACKET_TYPE_BARRIER_AND) { DPRINTF(HSAPacketProcessor, "%s: Processing barrier packet" \ " active list ID = %d\n", __FUNCTION__, rl_idx); @@ -631,6 +646,23 @@ HSAPacketProcessor::finishPkt(void *pvPkt, uint32_t rl_idx) { HSAQueueDescriptor* qDesc = regdQList[rl_idx]->qCntxt.qDesc; + +// if barrier bit was set, unset it here -- we assume that finishPkt is +// only called after the completion of a kernel +if (regdQList[rl_idx]->getBarrierBit()) { +DPRINTF(HSAPacketProcessor, +"Unset barrier bit for active list ID %d\n", rl_idx); +regdQList[rl_idx]->setBarrierBit(false); +// if pending kernels in the queue after this kernel, reschedule +if (regdQList[rl_idx]->dispPending()) { +DPRINTF(HSAPacketProcessor, +"Rescheduling active list ID %d after unsetting barrier " +"bit\n", rl_idx); +schedAQLProcessing(rl_idx); +} +} + +// If set, then blocked schedule, so need to reschedule if (regdQList[rl_idx]->qCntxt.aqlBuf->freeEntry(pvPkt)) updateReadIndex(0, rl_idx); DPRINTF(HSAPacketProcessor, diff --git a/src/dev/hsa/hsa_packet_processor.hh b/src/dev/hsa/hsa_packet_processor.hh index 206d9ab..3ff7ad2 100644 --- a/src/dev/hsa/hsa_packet_processor.hh +++ b/src/dev/hsa/hsa_packet_processor.hh @@ -168,11 +168,13 @@ typedef struct QueueContext { HSAQueueDescriptor* qDesc; AQLRingBuffer* aqlBuf; +// used for HSA packets that enforce synchronization with barrier bit +bool barrierBit; QueueContext(HSAQueueDescriptor* q_desc, AQLRingBuffer* aql_buf) - : qDesc(q_desc), aqlBuf(aql_buf) + : qDesc(q_desc), aqlBuf(aql_buf), barrierBit(false) {} -QueueContext() : qDesc(NULL), aqlBuf(NULL) {} +QueueContext() : qDesc(NULL), aqlBuf(NULL), barrierBit(false) {} } QCntxt; class HSAPacketPr
[gem5-dev] Change in gem5/gem5[develop]: gpu-compute: Make headTailMap a std::unordered_map
Anthony Gutierrez has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/29930 ) Change subject: gpu-compute: Make headTailMap a std::unordered_map .. gpu-compute: Make headTailMap a std::unordered_map There is no reason that the headTailMap needs to be sorted, so let's use a std::unordered_map. Change-Id: I18641b893352c18ec86e3775c8947a05a6c6547d Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/29930 Reviewed-by: Anthony Gutierrez Maintainer: Anthony Gutierrez Tested-by: kokoro --- M src/gpu-compute/compute_unit.hh 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Anthony Gutierrez: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/gpu-compute/compute_unit.hh b/src/gpu-compute/compute_unit.hh index 187cbc9..110097e 100644 --- a/src/gpu-compute/compute_unit.hh +++ b/src/gpu-compute/compute_unit.hh @@ -981,7 +981,7 @@ // hold the time of the arrival of the first cache block related to // a particular GPUDynInst. This is used to calculate the difference // between the first and last chace block arrival times. -std::map headTailMap; +std::unordered_map headTailMap; }; #endif // __COMPUTE_UNIT_HH__ -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/29930 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: I18641b893352c18ec86e3775c8947a05a6c6547d Gerrit-Change-Number: 29930 Gerrit-PatchSet: 6 Gerrit-Owner: Anthony Gutierrez Gerrit-Reviewer: Anthony Gutierrez Gerrit-Reviewer: Tony Gutierrez Gerrit-Reviewer: Tuan Ta 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-gcn3: Fix V_MAD_I32_I24 sign extension
Anthony Gutierrez has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/29928 ) Change subject: arch-gcn3: Fix V_MAD_I32_I24 sign extension .. arch-gcn3: Fix V_MAD_I32_I24 sign extension We are not properly sign extending the bits we hack off for V_MAD_I32_I24. This fixes rnn_fwdBwd 64 1 1 lstm pte assertion failure. Change-Id: I2516e5715227cbd822e6a62630674f64f7a109e0 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/29928 Reviewed-by: Anthony Gutierrez Reviewed-by: Matt Sinclair Maintainer: Anthony Gutierrez Tested-by: kokoro --- M src/arch/gcn3/insts/instructions.cc 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: Anthony Gutierrez: Looks good to me, approved; Looks good to me, approved Matt Sinclair: Looks good to me, approved kokoro: Regressions pass diff --git a/src/arch/gcn3/insts/instructions.cc b/src/arch/gcn3/insts/instructions.cc index 32719ad..0256d46 100644 --- a/src/arch/gcn3/insts/instructions.cc +++ b/src/arch/gcn3/insts/instructions.cc @@ -27446,8 +27446,8 @@ for (int lane = 0; lane < NumVecElemPerVecReg; ++lane) { if (wf->execMask(lane)) { -vdst[lane] = bits(src0[lane], 23, 0) * bits(src1[lane], 23, 0) -+ src2[lane]; +vdst[lane] = sext<24>(bits(src0[lane], 23, 0)) +* sext<24>(bits(src1[lane], 23, 0)) + src2[lane]; } } -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/29928 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: I2516e5715227cbd822e6a62630674f64f7a109e0 Gerrit-Change-Number: 29928 Gerrit-PatchSet: 6 Gerrit-Owner: Anthony Gutierrez Gerrit-Reviewer: Anthony Gutierrez Gerrit-Reviewer: Matt Sinclair Gerrit-Reviewer: Michael LeBeane Gerrit-Reviewer: Tony Gutierrez Gerrit-Reviewer: Tuan Ta 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-gcn3, gpu-compute: Fix issue when reading const operands
Anthony Gutierrez has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/29927 ) Change subject: arch-gcn3, gpu-compute: Fix issue when reading const operands .. arch-gcn3, gpu-compute: Fix issue when reading const operands Currently, when an instruction has an operand that reads a const value, it goes thru the same readMiscReg() api call as other misc registers (real HW registers, not constant values). There is an issue, however, when casting from the const values (which are 32b) to higher precision values, like 64b. This change creates a separate, templated function call to the GPU's ISA state that will return the correct type. Change-Id: I41965ebeeed20bb70e919fce5ad94d957b3af802 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/29927 Reviewed-by: Anthony Gutierrez Maintainer: Anthony Gutierrez Tested-by: kokoro --- M src/arch/gcn3/gpu_isa.hh M src/arch/gcn3/isa.cc M src/arch/gcn3/operand.hh M src/arch/gcn3/registers.cc M src/arch/gcn3/registers.hh M src/gpu-compute/gpu_exec_context.hh 6 files changed, 66 insertions(+), 17 deletions(-) Approvals: Anthony Gutierrez: Looks good to me, approved; Looks good to me, approved kokoro: Regressions pass diff --git a/src/arch/gcn3/gpu_isa.hh b/src/arch/gcn3/gpu_isa.hh index 26b79c7..228c3fe 100644 --- a/src/arch/gcn3/gpu_isa.hh +++ b/src/arch/gcn3/gpu_isa.hh @@ -37,6 +37,7 @@ #define __ARCH_GCN3_GPU_ISA_HH__ #include +#include #include "arch/gcn3/registers.hh" #include "gpu-compute/dispatcher.hh" @@ -52,6 +53,24 @@ public: GPUISA(Wavefront &wf); +template T +readConstVal(int opIdx) const +{ +panic_if(!std::is_integral::value, "Constant values must " + "be an integer.\n"); +T val(0); + +if (isPosConstVal(opIdx)) { +val = (T)readPosConstReg(opIdx); +} + +if (isNegConstVal(opIdx)) { +val = (T)readNegConstReg(opIdx); +} + +return val; +} + ScalarRegU32 readMiscReg(int opIdx) const; void writeMiscReg(int opIdx, ScalarRegU32 operandVal); bool hasScalarUnit() const { return true; } @@ -63,10 +82,9 @@ return posConstRegs[opIdx - REG_INT_CONST_POS_MIN]; } -ScalarRegU32 readNegConstReg(int opIdx) const +ScalarRegI32 readNegConstReg(int opIdx) const { -return *((ScalarRegU32*) -&negConstRegs[opIdx - REG_INT_CONST_NEG_MIN]); +return negConstRegs[opIdx - REG_INT_CONST_NEG_MIN]; } static const std::array diff --git a/src/arch/gcn3/isa.cc b/src/arch/gcn3/isa.cc index 036c771..3bd122d 100644 --- a/src/arch/gcn3/isa.cc +++ b/src/arch/gcn3/isa.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016-2017 Advanced Micro Devices, Inc. + * Copyright (c) 2016-2018 Advanced Micro Devices, Inc. * All rights reserved. * * For use for simulation and test purposes only @@ -49,14 +49,6 @@ ScalarRegU32 GPUISA::readMiscReg(int opIdx) const { -if (opIdx >= REG_INT_CONST_POS_MIN && opIdx <= REG_INT_CONST_POS_MAX) { -return readPosConstReg(opIdx); -} - -if (opIdx >= REG_INT_CONST_NEG_MIN && opIdx <= REG_INT_CONST_NEG_MAX) { -return readNegConstReg(opIdx); -} - switch (opIdx) { case REG_M0: return m0; diff --git a/src/arch/gcn3/operand.hh b/src/arch/gcn3/operand.hh index 218faf8..7f70fab 100644 --- a/src/arch/gcn3/operand.hh +++ b/src/arch/gcn3/operand.hh @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017 Advanced Micro Devices, Inc. + * Copyright (c) 2017-2018 Advanced Micro Devices, Inc. * All rights reserved. * * For use for simulation and test purposes only @@ -583,10 +583,15 @@ default: { assert(sizeof(DataType) <= sizeof(srfData)); -DataType misc_val -= (DataType)_gpuDynInst->readMiscReg(_opIdx); +DataType misc_val(0); +if (isConstVal(_opIdx)) { +misc_val = (DataType)_gpuDynInst +->readConstVal(_opIdx); +} else { +misc_val = (DataType)_gpuDynInst->readMiscReg(_opIdx); +} std::memcpy((void*)srfData.data(), (void*)&misc_val, -sizeof(DataType)); +sizeof(DataType)); } } } diff --git a/src/arch/gcn3/registers.cc b/src/arch/gcn3/registers.cc index 0872ff9..016160f 100644 --- a/src/arch/gcn3/registers.cc +++ b/src/arch/gcn3/registers.cc @@ -163,6 +163,31 @@ } bool +isPosConstVal(int opIdx) +{ +bool is_pos_const_val = (opIdx >= REG_INT_CONST_POS_MIN +
[gem5-dev] Call for review: Arm’s Transactional Memory Extension (TME)
Call for review: Arm’s Transactional Memory Extension (TME) We have recently uploaded a set of large patches that introduce (a)Partial support for hardware transactional memory (HTM) in gem5 (b)Architectural support in the Arm ISA for the Transactional Memory Extension (TME) HTM support is implemented in the Ruby memory system—specifically by modifying the MESI_Three_Level protocol. Various additions are made to the APIs of CPU, ExecContext and ThreadContext. The existing fault/exception system is reused to handle transactional aborts and architectural checkpoint restorations. Both TimingSimpleCPU and O3CPU are augmented to support general HTM functionality. HTM support is a reworking of a previous pull request from Pradip Vallathol from his master’s thesis done at The University of Wisconsin-Madison. https://gem5.atlassian.net/browse/GEM5-587 Arm’s TME ISA adds four new instructions: - TSTART – Creates an architectural checkpoint and places the CPU and cache hierarchy into transactional state in which all subsequent operations are executed speculatively and are not visible to the rest of the system until committed atomically. - TCOMMIT – Makes all speculative (transactional) operations globally visible and removes the CPU and cache hierarchy from transactional state. - TCANCEL – Discards all speculative memory operations, rolls back the architectural state to a checkpoint taken at TSTART and removes the CPU and cache hierarchy from transactional state. - TTEST – Returns the current transactional depth (e.g. if TSTART is called inside a transaction), or a zero if not in transactional state. https://gem5.atlassian.net/browse/GEM5-588 Gerrit patchset: https://gem5-review.googlesource.com/c/public/gem5/+/30314/1 IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: sim-se, util: Update MIOpen version used in Docker
Kyle Roarty has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/30494 ) Change subject: sim-se, util: Update MIOpen version used in Docker .. sim-se, util: Update MIOpen version used in Docker The updated MIOpen version uses rocBLAS instead of MIOpenGEMM for both convolution and rnn GEMM kernels, which provides a speedup in simulation. Set chmod to ignoreFunc as it caused crashes with this newer MIOpen version. Change-Id: I4b81f18e95d39fd79b22d0bf92563ede61e44e32 --- M src/arch/x86/linux/process.cc M util/dockerfiles/gcn-gpu/Dockerfile 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/arch/x86/linux/process.cc b/src/arch/x86/linux/process.cc index 3a86b76..b0e87c0 100644 --- a/src/arch/x86/linux/process.cc +++ b/src/arch/x86/linux/process.cc @@ -347,7 +347,7 @@ { 87, "unlink", unlinkFunc }, { 88, "symlink", symlinkFunc }, { 89, "readlink", readlinkFunc }, -{ 90, "chmod" }, +{ 90, "chmod", ignoreFunc }, { 91, "fchmod" }, { 92, "chown" }, { 93, "fchown" }, diff --git a/util/dockerfiles/gcn-gpu/Dockerfile b/util/dockerfiles/gcn-gpu/Dockerfile index 485a406..c499598 100644 --- a/util/dockerfiles/gcn-gpu/Dockerfile +++ b/util/dockerfiles/gcn-gpu/Dockerfile @@ -34,7 +34,7 @@ libboost-system-dev \ libboost-dev -ARG gem5_dist=http://dist.gem5.org/dist/current +ARG gem5_dist=http://dist.gem5.org/dist/develop # Install ROCm 1.6 binaries RUN wget -qO- ${gem5_dist}/apt_1.6.2.tar.bz2 \ @@ -61,14 +61,14 @@ RUN mkdir -p /patch && cd /patch && \ wget ${gem5_dist}/rocm_patches/hipBLAS.patch && \ wget ${gem5_dist}/rocm_patches/hip.patch && \ -wget ${gem5_dist}/rocm_patches/miopen.patch && \ +wget ${gem5_dist}/rocm_patches/miopen-conv.patch && \ wget ${gem5_dist}/rocm_patches/rocBLAS.patch RUN git -C /HIP/ checkout 0e3d824e && git -C /HIP/ apply /patch/hip.patch && \ git -C /hipBLAS/ checkout ee57787e && git -C /hipBLAS/ apply /patch/hipBLAS.patch && \ git -C /rocBLAS/ checkout cbff4b4e && git -C /rocBLAS/ apply /patch/rocBLAS.patch && \ git -C /MIOpenGEMM/ checkout 9547fb9e && \ -git -C /MIOpen/ checkout a9949e30 && git -C /MIOpen/ apply /patch/miopen.patch +git -C /MIOpen/ checkout 01d6ca55c && git -C /MIOpen/ apply /patch/miopen-conv.patch ENV ROCM_PATH /opt/rocm ENV HCC_HOME ${ROCM_PATH}/hcc @@ -107,6 +107,16 @@ # Should link this in as a volume if at all possible RUN mkdir -p /.cache/miopen && chmod 777 /.cache/miopen +# Un-set default c++ version for MIOpen compilation +# As MIOpen 1.7 requires c++14 or higher +RUN sed -i 's/INTERFACE_COMPILE_OPTIONS "-std=c++amp;-fPIC;-gline-tables-only"/#&/' /opt/rocm/hcc-1.0/lib/cmake/hcc/hcc-targets.cmake && \ +sed -i 's/INTERFACE_COMPILE_OPTIONS "-hc"/#&/' /opt/rocm/hcc-1.0/lib/cmake/hcc/hcc-targets.cmake + +WORKDIR /MIOpen +# Half is required; This is the version that MIOpen would download +RUN wget https://github.com/pfultz2/half/archive/1.12.0.tar.gz && \ +tar -xzf 1.12.0.tar.gz + WORKDIR /MIOpen/build RUN CXX=/opt/rocm/hcc/bin/hcc cmake \ -DCMAKE_BUILD_TYPE=Debug \ @@ -115,15 +125,21 @@ -DCMAKE_PREFIX_PATH="/opt/rocm/hip;/opt/rocm/hcc;/opt/rocm/rocdl;/opt/rocm/miopengemm;/opt/rocm/hsa" \ -DMIOPEN_CACHE_DIR=/.cache/miopen \ -DMIOPEN_AMDGCN_ASSEMBLER_PATH=/opt/rocm/opencl/bin \ +-DHALF_INCLUDE_DIR=/MIOpen/half-1.12.0/include \ -DCMAKE_CXX_FLAGS="-isystem /usr/include/x86_64-linux-gnu" .. && \ -make -j$(nproc) && make install && rm -rf * +make && make install && rm -rf * -# Create performance DB for gfx801. May need personal dbs still +# Re-set defaults +RUN sed -i 's/#\(INTERFACE_COMPILE_OPTIONS "-std=c++amp;-fPIC;-gline-tables-only"\)/\1/' /opt/rocm/hcc-1.0/lib/cmake/hcc/hcc-targets.cmake && \ +sed -i 's/#\(INTERFACE_COMPILE_OPTIONS "-hc"\)/\1/' /opt/rocm/hcc-1.0/lib/cmake/hcc/hcc-targets.cmake + +# Create performance DB for gfx801. WORKDIR /opt/rocm/miopen/share/miopen/db -RUN ln -s gfx803_64.cd.pdb.txt gfx801_8.cd.pdb.txt && \ -ln -s gfx803_64.cd.pdb.txt gfx801_16.cd.pdb.txt && \ -ln -s gfx803_64.cd.pdb.txt gfx801_32.cd.pdb.txt && \ -ln -s gfx803_64.cd.pdb.txt gfx801_64.cd.pdb.txt +RUN cp gfx803_64.cd.pdb.txt gfx801_4.cd.pdb.txt && \ +cp gfx803_64.cd.pdb.txt gfx801_8.cd.pdb.txt && \ +cp gfx803_64.cd.pdb.txt gfx801_16.cd.pdb.txt && \ +cp gfx803_64.cd.pdb.txt gfx801_32.cd.pdb.txt && \ +cp gfx803_64.cd.pdb.txt gfx801_64.cd.pdb.txt # Install profiler from .deb file, works for 1.6.2 WORKDIR /ROCm-Profiler -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/30494 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: I4b81f18e95d39fd79b22d0bf92563ede61e44e32 Gerrit-Change-Number:
[gem5-dev] bug squashing renamed pinned registers in o3?
Hi folks, specifically ARM folks. We've been seeing a problem with O3 where when switching vector register renaming modes (full vectors vs vector elements), the CPU checks its bookkeeping and finds that a vector register is missing, ie with no instructions in flight, the free list has one fewer register in it than the difference between the total number of physical vector registers, and the number that should be taken up with architectural state. This problem has been somewhat difficult to reproduce, although we can get it to happen, and it does happen often enough that it's been a real pain for us. Given that it's not very easy to get it to happen which makes it hard to observe, I've been digging around in the code trying to understand what all the pieces do and why the bookkeeping might be wrong. The most promising thing I've found so far is that when squashing, the rename stage looks at its history and rolls back renames for squashed instructions. Some registers are fixed and not renamed, so rolling back those would be pointless. Also those registers should not go on the free list. The way O3 detects those special registers is that they have the same index before and after renaming. If that is the case, O3 ignores those entries, and does not roll them back or mark their target as free. This check is slightly out of date though, since with the recently added pinned register writes, a register will be renamed to the same thing several times in a row. When these entries are checked, they will not be rolled back (I think this part is still fine), but they will also not be marked as free. This isn't exactly a smoking gun though, since the more I think about it, the more I think this may actually be ok. If one of the later writes is squashed, the register isn't "free" since it still holds the (partially written) architectural state. If everything gets squashed all the way back to the first entry which did change what register to use, then the slightly outdated check won't trigger and things should be freed up correctly (I think). This code is mostly new to me though, so I'm not super confident making any grand declarations about what's going on. All the pieces seem to be there though, which makes me very suspicious. Maybe something goes wrong if the right number of writes never happens because later writers get squashed? Gabe ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s