[gem5-dev] Change in gem5/gem5[develop]: mips,cpu: Eliminate the unused IsIndexed StaticInst flag.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/33735 )
Change subject: mips,cpu: Eliminate the unused IsIndexed StaticInst flag.
..
mips,cpu: Eliminate the unused IsIndexed StaticInst flag.
It's set by some MIPS instructions, but does not have an accessor in
StaticInst and is not used by anything.
Change-Id: I3466f7d2723fb1b0ac195064867e3840e3a8f21b
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/33735
Reviewed-by: Bobby R. Bruce
Reviewed-by: Jason Lowe-Power
Maintainer: Gabe Black
Tested-by: kokoro
---
M src/arch/mips/isa/formats/mem.isa
M src/cpu/StaticInstFlags.py
2 files changed, 2 insertions(+), 6 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, but someone else must approve
Bobby R. Bruce: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/mips/isa/formats/mem.isa
b/src/arch/mips/isa/formats/mem.isa
index 491dd0c..c5fd6f8 100644
--- a/src/arch/mips/isa/formats/mem.isa
+++ b/src/arch/mips/isa/formats/mem.isa
@@ -458,7 +458,6 @@
def format LoadIndexedMemory(memacc_code, ea_code = {{ EA = Rs + Rt; }},
mem_flags = [], inst_flags = []) {{
-inst_flags += ['IsIndexed']
(header_output, decoder_output, decode_block, exec_output) = \
LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags,
inst_flags,
decode_template = ImmNopCheckDecode,
@@ -467,7 +466,6 @@
def format StoreIndexedMemory(memacc_code, ea_code = {{ EA = Rs + Rt; }},
mem_flags = [], inst_flags = []) {{
-inst_flags += ['IsIndexed']
(header_output, decoder_output, decode_block, exec_output) = \
LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags,
inst_flags,
exec_template_base = 'Store')
@@ -475,7 +473,7 @@
def format LoadFPIndexedMemory(memacc_code, ea_code = {{ EA = Rs + Rt; }},
mem_flags = [], inst_flags = []) {{
-inst_flags += ['IsIndexed', 'IsFloating']
+inst_flags += ['IsFloating']
(header_output, decoder_output, decode_block, exec_output) = \
LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags,
inst_flags,
decode_template = ImmNopCheckDecode,
@@ -484,7 +482,7 @@
def format StoreFPIndexedMemory(memacc_code, ea_code = {{ EA = Rs + Rt; }},
mem_flags = [], inst_flags = []) {{
-inst_flags += ['IsIndexed', 'IsFloating']
+inst_flags += ['IsFloating']
(header_output, decoder_output, decode_block, exec_output) = \
LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags,
inst_flags,
exec_template_base = 'Store')
diff --git a/src/cpu/StaticInstFlags.py b/src/cpu/StaticInstFlags.py
index 8db5443..cbdfec3 100644
--- a/src/cpu/StaticInstFlags.py
+++ b/src/cpu/StaticInstFlags.py
@@ -64,8 +64,6 @@
'IsStore', # Writes to memory.
'IsAtomic', # Does atomic RMW to memory.
'IsStoreConditional', # Store conditional instruction.
-'IsIndexed',# Accesses memory with an indexed address
-# computation
'IsInstPrefetch', # Instruction-cache prefetch.
'IsDataPrefetch', # Data-cache prefetch.
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/33735
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: I3466f7d2723fb1b0ac195064867e3840e3a8f21b
Gerrit-Change-Number: 33735
Gerrit-PatchSet: 6
Gerrit-Owner: Gabe Black
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Bobby R. Bruce
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[release-staging-v20.1.0.0]: configs: Add special case in MemConfig
Jason Lowe-Power has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/34595 ) Change subject: configs: Add special case in MemConfig .. configs: Add special case in MemConfig SimpleMemory doesn't implement a full MemCtrl interface. Thus, like the NVM and HMC memories, we need to add a special case to MemConfig.py. The --mem-type command line option now works for SimpleMemory and all of the DRAM interfaces (it does not work for the NVM interfaces, though). Change-Id: I6d60649215be324bdd2a104b1976752f936c960e Signed-off-by: Jason Lowe-Power --- M configs/common/MemConfig.py 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/configs/common/MemConfig.py b/configs/common/MemConfig.py index 941b381..224ddc7 100644 --- a/configs/common/MemConfig.py +++ b/configs/common/MemConfig.py @@ -177,6 +177,9 @@ if opt_nvm_type: n_intf = ObjectList.mem_list.get(opt_nvm_type) +print(opt_mem_type) +ObjectList.mem_list.print() + nvm_intfs = [] mem_ctrls = [] @@ -227,11 +230,15 @@ mem_ctrl = m5.objects.MemCtrl(min_writes_per_switch = 8, static_backend_latency = '4ns', static_frontend_latency = '4ns') +elif opt_mem_type == "SimpleMemory": +mem_ctrl = m5.objects.SimpleMemory() else: mem_ctrl = m5.objects.MemCtrl() # Hookup the controller to the interface and add to the list -mem_ctrl.dram = dram_intf +if opt_mem_type != "SimpleMemory": +mem_ctrl.dram = dram_intf + mem_ctrls.append(mem_ctrl) elif opt_nvm_type and (not opt_mem_type or range_iter % 2 == 0): -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34595 To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem5 Gerrit-Branch: release-staging-v20.1.0.0 Gerrit-Change-Id: I6d60649215be324bdd2a104b1976752f936c960e Gerrit-Change-Number: 34595 Gerrit-PatchSet: 1 Gerrit-Owner: Jason Lowe-Power 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[release-staging-v20.1.0.0]: gpu: Fix a syntax error in X86GPUTLB.py.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34576 )
Change subject: gpu: Fix a syntax error in X86GPUTLB.py.
..
gpu: Fix a syntax error in X86GPUTLB.py.
The recent changes which removed master/slave terminology also
accidentally deleted an "=", making the syntax in that file illegal.
Change-Id: I50aa945f0f66765db36775380b98a88caff23c13
---
M src/gpu-compute/X86GPUTLB.py
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/gpu-compute/X86GPUTLB.py b/src/gpu-compute/X86GPUTLB.py
index 45cb962..fee9b9a 100644
--- a/src/gpu-compute/X86GPUTLB.py
+++ b/src/gpu-compute/X86GPUTLB.py
@@ -77,6 +77,6 @@
slave= DeprecatedParam(cpu_side_ports,
'`slave` is now called `cpu_side_ports`')
mem_side_ports = VectorRequestPort("Port on side closer to memory")
-master DeprecatedParam(mem_side_ports,
+master = DeprecatedParam(mem_side_ports,
'`master` is now called `mem_side_ports`')
disableCoalescing = Param.Bool(False,"Dispable Coalescing")
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34576
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v20.1.0.0
Gerrit-Change-Id: I50aa945f0f66765db36775380b98a88caff23c13
Gerrit-Change-Number: 34576
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[release-staging-v20.1.0.0]: arm: Use zero initialization for the BigRegVect types.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34575 )
Change subject: arm: Use zero initialization for the BigRegVect types.
..
arm: Use zero initialization for the BigRegVect types.
These were being initialized with BigRegVect brv = {0}, which made the
compiler complain because there is internal structure. The first element
of the union is actually an array, and this was telling it to initialize
that array to scalar 0. It was warning about this which was breaking the
build.
Instead, use zero initlization like BigRegVect brv = {}. This
initializes the first element of the union to all zeroes, with all
padding bits initialized to zero as well.
This satisfies the compiler and avoids a build error.
Change-Id: I31e7a8730c538637ff2e0c7fb00a4e12ed05e074
---
M src/arch/arm/isa/insts/neon.isa
M src/arch/arm/isa/insts/neon64.isa
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/arch/arm/isa/insts/neon.isa
b/src/arch/arm/isa/insts/neon.isa
index c8f8fcd..6290203 100644
--- a/src/arch/arm/isa/insts/neon.isa
+++ b/src/arch/arm/isa/insts/neon.isa
@@ -1452,7 +1452,7 @@
rCount = 2
eWalkCode = simdEnabledCheckCode + '''
RegVect srcReg1, srcReg2;
-BigRegVect destReg = {0};
+BigRegVect destReg = {};
'''
for reg in range(rCount):
eWalkCode += '''
@@ -1654,7 +1654,7 @@
global header_output, exec_output
eWalkCode = simdEnabledCheckCode + '''
RegVect srcReg1;
-BigRegVect destReg = {0};
+BigRegVect destReg = {};
'''
for reg in range(2):
eWalkCode += '''
@@ -1884,7 +1884,7 @@
global header_output, exec_output
eWalkCode = simdEnabledCheckCode + '''
RegVect srcRegs;
-BigRegVect destReg = {0};
+BigRegVect destReg = {};
'''
for reg in range(rCount):
eWalkCode += '''
@@ -2010,7 +2010,7 @@
global header_output, exec_output
eWalkCode = simdEnabledCheckCode + '''
RegVect srcReg1;
-BigRegVect destReg = {0};
+BigRegVect destReg = {};
'''
for reg in range(2):
eWalkCode += '''
diff --git a/src/arch/arm/isa/insts/neon64.isa
b/src/arch/arm/isa/insts/neon64.isa
index 702c128..f049c3e 100644
--- a/src/arch/arm/isa/insts/neon64.isa
+++ b/src/arch/arm/isa/insts/neon64.isa
@@ -351,7 +351,7 @@
global header_output, exec_output
eWalkCode = simd64EnabledCheckCode + '''
RegVect srcReg1;
-BigRegVect destReg = {0};
+BigRegVect destReg = {};
'''
destReg = 0 if not hi else 2
for reg in range(2):
@@ -632,7 +632,7 @@
global header_output, exec_output
eWalkCode = simd64EnabledCheckCode + '''
RegVect srcRegs;
-BigRegVect destReg = {0};
+BigRegVect destReg = {};
'''
for reg in range(rCount):
eWalkCode += '''
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34575
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v20.1.0.0
Gerrit-Change-Id: I31e7a8730c538637ff2e0c7fb00a4e12ed05e074
Gerrit-Change-Number: 34575
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[release-staging-v20.1.0.0]: mem: Remove conditional includes based on THE_ISA in ruby.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/34577 ) Change subject: mem: Remove conditional includes based on THE_ISA in ruby. .. mem: Remove conditional includes based on THE_ISA in ruby. These were including instruction class definitions from x86 for some reason. There was no code in those .cc files which actually used anything from them, as evidenced by the fact that the GCN3_X86 build still works. No other code in the file was conditionally compiled as of today. Change-Id: I3cef8348fb601dd7af67665cf64bbf514c91c3db --- M src/mem/ruby/system/GPUCoalescer.cc M src/mem/ruby/system/VIPERCoalescer.cc 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/mem/ruby/system/GPUCoalescer.cc b/src/mem/ruby/system/GPUCoalescer.cc index 03c392f..f5d4f02 100644 --- a/src/mem/ruby/system/GPUCoalescer.cc +++ b/src/mem/ruby/system/GPUCoalescer.cc @@ -31,16 +31,11 @@ * POSSIBILITY OF SUCH DAMAGE. */ +#include "mem/ruby/system/GPUCoalescer.hh" + #include "base/logging.hh" #include "base/str.hh" #include "config/the_isa.hh" - -#if THE_ISA == X86_ISA -#include "arch/x86/insts/microldstop.hh" - -#endif // X86_ISA -#include "mem/ruby/system/GPUCoalescer.hh" - #include "cpu/testers/rubytest/RubyTester.hh" #include "debug/GPUCoalescer.hh" #include "debug/MemoryAccess.hh" diff --git a/src/mem/ruby/system/VIPERCoalescer.cc b/src/mem/ruby/system/VIPERCoalescer.cc index a8a3aa9..82c7f00 100644 --- a/src/mem/ruby/system/VIPERCoalescer.cc +++ b/src/mem/ruby/system/VIPERCoalescer.cc @@ -31,16 +31,11 @@ * POSSIBILITY OF SUCH DAMAGE. */ +#include "mem/ruby/system/VIPERCoalescer.hh" + #include "base/logging.hh" #include "base/str.hh" #include "config/the_isa.hh" - -#if THE_ISA == X86_ISA -#include "arch/x86/insts/microldstop.hh" - -#endif // X86_ISA -#include "mem/ruby/system/VIPERCoalescer.hh" - #include "cpu/testers/rubytest/RubyTester.hh" #include "debug/GPUCoalescer.hh" #include "debug/MemoryAccess.hh" -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34577 To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings Gerrit-Project: public/gem5 Gerrit-Branch: release-staging-v20.1.0.0 Gerrit-Change-Id: I3cef8348fb601dd7af67665cf64bbf514c91c3db Gerrit-Change-Number: 34577 Gerrit-PatchSet: 1 Gerrit-Owner: Gabe Black Gerrit-MessageType: newchange ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Re: MessageBuffer double counting delay between arrival and dequeue ticks?
That's fine with me! I love removing code! Cheers, Jason On Tue, Sep 15, 2020 at 1:08 PM Gambord, Ryan via gem5-dev < gem5-dev@gem5.org> wrote: > Hi Srikant, > > Thank you for looking into it. I noticed the miscalculation while working > on something unrelated, and wasn't sure what those values were used for (I > don't need them). If it's not being used, then my proposed change would be > to remove it from the codebase as legacy bloat. Does anyone have a problem > with that? > > Ryan Gambord > > > > > On Mon, Sep 14, 2020 at 11:44 PM Srikant Bharadwaj via gem5-dev < > gem5-dev@gem5.org> wrote: > >> [This email originated from outside of OSU. Use caution with links and >> attachments.] >> Hi Ryan, >> You are right. If the message goes to the next message buffer, delay is >> added again. >> However, as far as I know we are not using the delayed ticks for >> calculating anything anymore. The earlier use case was to calculate the >> stall time within MessageBuffer, but we use the getTime to do that now. >> Is there any specific use case you are trying to fix here? >> >> In any case, it would be great if you can post a fix. >> >> Thanks, >> Srikant >> >> On Mon, Sep 14, 2020 at 10:48 AM Jason Lowe-Power via gem5-dev < >> gem5-dev@gem5.org> wrote: >> >>> Hey Ryan, >>> >>> Sorry for the slow reply. Yes, it looks like delayedTicks may be double >>> counting in some cases. I wonder if a little microbenchmark might be able >>> to confirm more clearly? Assuming it is being double counted, we'd welcome >>> a fix! >>> >>> @Bharadwaj, Srikant might be able to help >>> as well. Srikant, did you see anything like this with Garnet 3.0? >>> >>> Cheers, >>> Jason >>> >>> On Fri, Sep 11, 2020 at 2:35 AM Gambord, Ryan via gem5-dev < >>> gem5-dev@gem5.org> wrote: >>> bump On Tue, Sep 1, 2020, 12:21 Gambord, Ryan wrote: > I noticed that MessageBuffer calls UpdateDelayedTicks in both enqueue > and dequeue methods. Since dequeue does not setLastEnqueueTime of the > message to the time it was dequeued at, when enqueue calls > UpdateDelayedTicks, doesn't it add the dequeue delay to the delayed ticks > a > second time? > > Below is a table of the timeline. X and Y are the starting values for > LastEnqueueTime and DelayedTicks when the first message buffer receives > the > message. When the message is dequeued from MB1, DelayedTicks gets C-B > added > to it. When it is then enqueued in MB2, it gets D-B added, which double > counts the C-B interval. > > curTime() FunctionCall m_LastEnqueueTime m_DelayedTicks > > > X Y > A enqueue() B = A + Delta Y + (A-X) > B wakeup() " " > C dequeue() " Y + (A-X) + (C-B) > D enqueue() E = D + Delta > Y + (A-X) + (C-B) + (D-B) = Y + (A-X) + (C-B) + (D-C) + (C-B) > > > Ryan Gambord > > > ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s >>> >>> ___ >>> gem5-dev mailing list -- gem5-dev@gem5.org >>> To unsubscribe send an email to gem5-dev-le...@gem5.org >>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s >> >> ___ >> gem5-dev mailing list -- gem5-dev@gem5.org >> To unsubscribe send an email to gem5-dev-le...@gem5.org >> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s > > ___ > gem5-dev mailing list -- gem5-dev@gem5.org > To unsubscribe send an email to gem5-dev-le...@gem5.org > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: cpu: Get rid of the unused IsMicroBranch StaticInst flag.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/33742 )
Change subject: cpu: Get rid of the unused IsMicroBranch StaticInst flag.
..
cpu: Get rid of the unused IsMicroBranch StaticInst flag.
This flag was never set, nor read.
Change-Id: I74506c220d96b53dcd44740639286b1dbbe84d2e
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/33742
Tested-by: kokoro
Reviewed-by: Gabe Black
Maintainer: Gabe Black
---
M src/cpu/StaticInstFlags.py
M src/cpu/base_dyn_inst.hh
M src/cpu/static_inst.hh
3 files changed, 0 insertions(+), 6 deletions(-)
Approvals:
Gabe Black: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/cpu/StaticInstFlags.py b/src/cpu/StaticInstFlags.py
index 316aef4..8db5443 100644
--- a/src/cpu/StaticInstFlags.py
+++ b/src/cpu/StaticInstFlags.py
@@ -99,9 +99,6 @@
'IsDelayedCommit', # This microop doesn't commit right away
'IsLastMicroop',# This microop ends a microop sequence
'IsFirstMicroop', # This microop begins a microop sequence
-# This flag doesn't do anything yet
-'IsMicroBranch',# This microop branches within the microcode
for
-# a macroop
'IsSquashAfter', # Squash all uncommitted state after executed
diff --git a/src/cpu/base_dyn_inst.hh b/src/cpu/base_dyn_inst.hh
index 00639ad..64ed060 100644
--- a/src/cpu/base_dyn_inst.hh
+++ b/src/cpu/base_dyn_inst.hh
@@ -564,7 +564,6 @@
bool isDelayedCommit() const { return staticInst->isDelayedCommit(); }
bool isLastMicroop() const { return staticInst->isLastMicroop(); }
bool isFirstMicroop() const { return staticInst->isFirstMicroop(); }
-bool isMicroBranch() const { return staticInst->isMicroBranch(); }
// hardware transactional memory
bool isHtmStart() const { return staticInst->isHtmStart(); }
bool isHtmStop() const { return staticInst->isHtmStop(); }
diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh
index 0a871cf..f77193a 100644
--- a/src/cpu/static_inst.hh
+++ b/src/cpu/static_inst.hh
@@ -196,8 +196,6 @@
bool isDelayedCommit() const { return flags[IsDelayedCommit]; }
bool isLastMicroop() const { return flags[IsLastMicroop]; }
bool isFirstMicroop() const { return flags[IsFirstMicroop]; }
-//This flag doesn't do anything yet
-bool isMicroBranch() const { return flags[IsMicroBranch]; }
// hardware transactional memory
// HtmCmds must be identified as such in order
// to provide them with necessary memory ordering semantics.
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/33742
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: I74506c220d96b53dcd44740639286b1dbbe84d2e
Gerrit-Change-Number: 33742
Gerrit-PatchSet: 7
Gerrit-Owner: Gabe Black
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Giacomo Travaglini
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: kokoro
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: x86,cpu: Get rid of the unused IsCC StaticInst flag.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/33741 )
Change subject: x86,cpu: Get rid of the unused IsCC StaticInst flag.
..
x86,cpu: Get rid of the unused IsCC StaticInst flag.
This flag was set when some registers were used in x86, but never
actually checked by anything.
Change-Id: Id0f9847aeca5017455929ab4bbf28210288a3553
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/33741
Reviewed-by: Jason Lowe-Power
Tested-by: kokoro
Maintainer: Gabe Black
---
M src/arch/x86/isa/operands.isa
M src/cpu/StaticInstFlags.py
M src/cpu/static_inst.hh
3 files changed, 6 insertions(+), 8 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/x86/isa/operands.isa b/src/arch/x86/isa/operands.isa
index 2cd92dd..64c83c6 100644
--- a/src/arch/x86/isa/operands.isa
+++ b/src/arch/x86/isa/operands.isa
@@ -64,7 +64,7 @@
def floatReg(idx, id):
return ('FloatReg', 'df', idx, 'IsFloating', id)
def ccReg(idx, id):
-return ('CCReg', 'uqw', idx, 'IsCC', id)
+return ('CCReg', 'uqw', idx, None, id)
def controlReg(idx, id, ctype = 'uqw'):
return ('ControlReg', ctype, idx,
(None, None, ['IsSerializeAfter',
@@ -147,20 +147,20 @@
# would be retained, the write predicate checks if any of the bits
# are being written.
-'PredccFlagBits': ('CCReg', 'uqw', '(CCREG_ZAPS)', 'IsCC',
+'PredccFlagBits': ('CCReg', 'uqw', '(CCREG_ZAPS)', None,
60, None, None, '''(((ext & (PFBit | AFBit | ZFBit | SFBit
)) != (PFBit | AFBit | ZFBit | SFBit )) &&
((ext & (PFBit | AFBit | ZFBit | SFBit )) != 0))''',
'((ext & (PFBit | AFBit | ZFBit | SFBit )) != 0)'),
-'PredcfofBits': ('CCReg', 'uqw', '(CCREG_CFOF)', 'IsCC',
+'PredcfofBits': ('CCReg', 'uqw', '(CCREG_CFOF)', None,
61, None, None, '''(((ext & CFBit) == 0 ||
(ext & OFBit) == 0) && ((ext & (CFBit | OFBit)) != 0))''',
'((ext & (CFBit | OFBit)) != 0)'),
-'PreddfBit': ('CCReg', 'uqw', '(CCREG_DF)', 'IsCC',
+'PreddfBit': ('CCReg', 'uqw', '(CCREG_DF)', None,
62, None, None, '(false)', '((ext & DFBit) != 0)'),
-'PredecfBit': ('CCReg', 'uqw', '(CCREG_ECF)', 'IsCC',
+'PredecfBit': ('CCReg', 'uqw', '(CCREG_ECF)', None,
63, None, None, '(false)', '((ext & ECFBit) != 0)'),
-'PredezfBit': ('CCReg', 'uqw', '(CCREG_EZF)', 'IsCC',
+'PredezfBit': ('CCReg', 'uqw', '(CCREG_EZF)', None,
64, None, None, '(false)', '((ext & EZFBit) != 0)'),
# These register should needs to be more protected so that later
diff --git a/src/cpu/StaticInstFlags.py b/src/cpu/StaticInstFlags.py
index 27ba013..316aef4 100644
--- a/src/cpu/StaticInstFlags.py
+++ b/src/cpu/StaticInstFlags.py
@@ -56,7 +56,6 @@
'IsInteger',# References integer regs.
'IsFloating', # References FP regs.
-'IsCC', # References CC regs.
'IsVector', # References Vector regs.
'IsVectorElem', # References Vector reg elems.
diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh
index 353c0e3..0a871cf 100644
--- a/src/cpu/static_inst.hh
+++ b/src/cpu/static_inst.hh
@@ -170,7 +170,6 @@
bool isInteger() const { return flags[IsInteger]; }
bool isFloating() const { return flags[IsFloating]; }
bool isVector() const { return flags[IsVector]; }
-bool isCC() const { return flags[IsCC]; }
bool isControl() const { return flags[IsControl]; }
bool isCall() const { return flags[IsCall]; }
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/33741
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: Id0f9847aeca5017455929ab4bbf28210288a3553
Gerrit-Change-Number: 33741
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Bobby R. Bruce
Gerrit-Reviewer: Brandon Potter
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Giacomo Travaglini
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: Matthew Poremba
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: Remove #if THE_ISA in the AbstractMemory class.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/34499 )
Change subject: mem: Remove #if THE_ISA in the AbstractMemory class.
..
mem: Remove #if THE_ISA in the AbstractMemory class.
This used to guard the extraction of the endianness when tracing memory
accesses. Since that's now always possible even in NULL_ISA, we don't
need conditional compilation.
Change-Id: Ie5ec76f5b0f27dd4123bc0f0a4c02438bed629ad
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34499
Tested-by: kokoro
Reviewed-by: Jason Lowe-Power
Reviewed-by: Nikos Nikoleris
Maintainer: Gabe Black
---
M src/mem/abstract_mem.cc
1 file changed, 1 insertion(+), 3 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Nikos Nikoleris: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/mem/abstract_mem.cc b/src/mem/abstract_mem.cc
index 79f716c..a708c6c 100644
--- a/src/mem/abstract_mem.cc
+++ b/src/mem/abstract_mem.cc
@@ -345,17 +345,15 @@
tracePacket(System *sys, const char *label, PacketPtr pkt)
{
int size = pkt->getSize();
-#if THE_ISA != NULL_ISA
if (size == 1 || size == 2 || size == 4 || size == 8) {
ByteOrder byte_order = sys->getGuestByteOrder();
-DPRINTF(MemoryAccess,"%s from %s of size %i on address %#x data "
+DPRINTF(MemoryAccess, "%s from %s of size %i on address %#x data "
"%#x %c\n", label, sys->getRequestorName(pkt->req->
requestorId()), size, pkt->getAddr(),
size, pkt->getAddr(), pkt->getUintX(byte_order),
pkt->req->isUncacheable() ? 'U' : 'C');
return;
}
-#endif
DPRINTF(MemoryAccess, "%s from %s of size %i on address %#x %c\n",
label, sys->getRequestorName(pkt->req->requestorId()),
size, pkt->getAddr(), pkt->req->isUncacheable() ? 'U' : 'C');
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34499
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: Ie5ec76f5b0f27dd4123bc0f0a4c02438bed629ad
Gerrit-Change-Number: 34499
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: Nikos Nikoleris
Gerrit-Reviewer: kokoro
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: mips,cpu: Get rid of the IsDpsOp StaticInst flag.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/33740 )
Change subject: mips,cpu: Get rid of the IsDpsOp StaticInst flag.
..
mips,cpu: Get rid of the IsDpsOp StaticInst flag.
This flag was set by MIPS for a few instructions, but didn't have an
accessor in StaticInst and was never used for anything.
Change-Id: I153cedde0d16cb1d78b2705bd7340ebfd10e4fb6
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/33740
Reviewed-by: Gabe Black
Maintainer: Gabe Black
Tested-by: kokoro
---
M src/arch/mips/isa/formats/dsp.isa
M src/cpu/StaticInstFlags.py
2 files changed, 2 insertions(+), 5 deletions(-)
Approvals:
Gabe Black: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/mips/isa/formats/dsp.isa
b/src/arch/mips/isa/formats/dsp.isa
index 9a6d614..12af2d6 100644
--- a/src/arch/mips/isa/formats/dsp.isa
+++ b/src/arch/mips/isa/formats/dsp.isa
@@ -173,8 +173,6 @@
code = decl_code + code + write_code
-opt_flags += ('IsDspOp',)
-
iop = InstObjParams(name, Name, 'DspIntOp', code, opt_flags)
header_output = BasicDeclare.subst(iop)
decoder_output = BasicConstructor.subst(iop)
@@ -204,8 +202,6 @@
code = decl_code + fetch_code + code + write_code
-opt_flags += ('IsDspOp',)
-
iop = InstObjParams(name, Name, 'DspHiLoOp', code, opt_flags)
header_output = BasicDeclare.subst(iop)
decoder_output = BasicConstructor.subst(iop)
diff --git a/src/cpu/StaticInstFlags.py b/src/cpu/StaticInstFlags.py
index 151074e..27ba013 100644
--- a/src/cpu/StaticInstFlags.py
+++ b/src/cpu/StaticInstFlags.py
@@ -103,8 +103,9 @@
# This flag doesn't do anything yet
'IsMicroBranch',# This microop branches within the microcode
for
# a macroop
-'IsDspOp',
+
'IsSquashAfter', # Squash all uncommitted state after executed
+
# hardware transactional memory
'IsHtmStart', # Starts a HTM transaction
'IsHtmStop',# Stops (commits) a HTM transaction
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/33740
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: I153cedde0d16cb1d78b2705bd7340ebfd10e4fb6
Gerrit-Change-Number: 33740
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Bobby R. Bruce
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] Re: MessageBuffer double counting delay between arrival and dequeue ticks?
Hi Srikant, Thank you for looking into it. I noticed the miscalculation while working on something unrelated, and wasn't sure what those values were used for (I don't need them). If it's not being used, then my proposed change would be to remove it from the codebase as legacy bloat. Does anyone have a problem with that? Ryan Gambord On Mon, Sep 14, 2020 at 11:44 PM Srikant Bharadwaj via gem5-dev < gem5-dev@gem5.org> wrote: > [This email originated from outside of OSU. Use caution with links and > attachments.] > Hi Ryan, > You are right. If the message goes to the next message buffer, delay is > added again. > However, as far as I know we are not using the delayed ticks for > calculating anything anymore. The earlier use case was to calculate the > stall time within MessageBuffer, but we use the getTime to do that now. > Is there any specific use case you are trying to fix here? > > In any case, it would be great if you can post a fix. > > Thanks, > Srikant > > On Mon, Sep 14, 2020 at 10:48 AM Jason Lowe-Power via gem5-dev < > gem5-dev@gem5.org> wrote: > >> Hey Ryan, >> >> Sorry for the slow reply. Yes, it looks like delayedTicks may be double >> counting in some cases. I wonder if a little microbenchmark might be able >> to confirm more clearly? Assuming it is being double counted, we'd welcome >> a fix! >> >> @Bharadwaj, Srikant might be able to help as >> well. Srikant, did you see anything like this with Garnet 3.0? >> >> Cheers, >> Jason >> >> On Fri, Sep 11, 2020 at 2:35 AM Gambord, Ryan via gem5-dev < >> gem5-dev@gem5.org> wrote: >> >>> bump >>> >>> On Tue, Sep 1, 2020, 12:21 Gambord, Ryan >>> wrote: >>> I noticed that MessageBuffer calls UpdateDelayedTicks in both enqueue and dequeue methods. Since dequeue does not setLastEnqueueTime of the message to the time it was dequeued at, when enqueue calls UpdateDelayedTicks, doesn't it add the dequeue delay to the delayed ticks a second time? Below is a table of the timeline. X and Y are the starting values for LastEnqueueTime and DelayedTicks when the first message buffer receives the message. When the message is dequeued from MB1, DelayedTicks gets C-B added to it. When it is then enqueued in MB2, it gets D-B added, which double counts the C-B interval. curTime() FunctionCall m_LastEnqueueTime m_DelayedTicks X Y A enqueue() B = A + Delta Y + (A-X) B wakeup() " " C dequeue() " Y + (A-X) + (C-B) D enqueue() E = D + Delta Y + (A-X) + (C-B) + (D-B) = Y + (A-X) + (C-B) + (D-C) + (C-B) Ryan Gambord ___ >>> gem5-dev mailing list -- gem5-dev@gem5.org >>> To unsubscribe send an email to gem5-dev-le...@gem5.org >>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s >> >> ___ >> gem5-dev mailing list -- gem5-dev@gem5.org >> To unsubscribe send an email to gem5-dev-le...@gem5.org >> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s > > ___ > gem5-dev mailing list -- gem5-dev@gem5.org > To unsubscribe send an email to gem5-dev-le...@gem5.org > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s ___ gem5-dev 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[release-staging-v20.1.0.0]: cpu,misc: Revert problematic terminology renames in BaseCPU
Bobby R. Bruce has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/34495 )
Change subject: cpu,misc: Revert problematic terminology renames in BaseCPU
..
cpu,misc: Revert problematic terminology renames in BaseCPU
Due to gem5's use of duck-typing, we must termorarly revert the
terminology in BaseCPU back to master/slave to avoid issues.
This fixes https://gem5.atlassian.net/browse/GEM5-775.
Change-Id: Idf1cb99aa9568ee70943ebec96f27394d8167f8c
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34495
Reviewed-by: Bobby R. Bruce
Reviewed-by: Jason Lowe-Power
Maintainer: Bobby R. Bruce
Tested-by: kokoro
---
M src/cpu/BaseCPU.py
1 file changed, 3 insertions(+), 3 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/cpu/BaseCPU.py b/src/cpu/BaseCPU.py
index c9e8ae6..ad91f3a 100644
--- a/src/cpu/BaseCPU.py
+++ b/src/cpu/BaseCPU.py
@@ -194,13 +194,13 @@
def connectCachedPorts(self, bus):
for p in self._cached_ports:
-exec('self.%s = bus.cpu_side_ports' % p)
+exec('self.%s = bus.slave' % p)
def connectUncachedPorts(self, bus):
for p in self._uncached_interrupt_response_ports:
-exec('self.%s = bus.mem_side_ports' % p)
+exec('self.%s = bus.master' % p)
for p in self._uncached_interrupt_request_ports:
-exec('self.%s = bus.cpu_side_ports' % p)
+exec('self.%s = bus.slave' % p)
def connectAllPorts(self, cached_bus, uncached_bus = None):
self.connectCachedPorts(cached_bus)
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34495
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v20.1.0.0
Gerrit-Change-Id: Idf1cb99aa9568ee70943ebec96f27394d8167f8c
Gerrit-Change-Number: 34495
Gerrit-PatchSet: 2
Gerrit-Owner: Bobby R. Bruce
Gerrit-Reviewer: Bobby R. Bruce
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]: gpu-compute: Fix deadlock in fetch_unit after branch instruction
Kyle Roarty has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34555 )
Change subject: gpu-compute: Fix deadlock in fetch_unit after branch
instruction
..
gpu-compute: Fix deadlock in fetch_unit after branch instruction
The following deadlock was occuring in fetch_unit w/timingSim:
1. exec() is called, a wave is ready to fetch, so it sets pendingFetch
2. A packet is sent to ITLB to fetch for that wave
3. The wave executes a branch, causing the fetch buffer to be cleared
4. The packet is handled, and fetch() is called. However, because the
fetch buffer was cleared, it returns doing nothing.
5. exec() gets called again, but the wave will never be scheduled to
fetch, as pendingFetch is still set to true.
This patch clears pendingFetch (and dropFetch) before returning when
the instruction buffer has been cleared in fetch().
dropFetch needed to be cleared otherwise gem5 would crash.
Change-Id: Iccbac7defc4849c19e8b17aa2492da641defb772
---
M src/gpu-compute/fetch_unit.cc
1 file changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/gpu-compute/fetch_unit.cc b/src/gpu-compute/fetch_unit.cc
index 4e4259e..098b783 100644
--- a/src/gpu-compute/fetch_unit.cc
+++ b/src/gpu-compute/fetch_unit.cc
@@ -240,6 +240,8 @@
* pending, in the same cycle another instruction is trying to fetch.
*/
if
(!fetchBuf.at(wavefront->wfSlotId).isReserved(pkt->req->getVaddr())) {
+wavefront->dropFetch = false;
+wavefront->pendingFetch = false;
return;
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34555
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: Iccbac7defc4849c19e8b17aa2492da641defb772
Gerrit-Change-Number: 34555
Gerrit-PatchSet: 1
Gerrit-Owner: Kyle Roarty
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Change in gem5/gem5[develop]: systemc: avoid dynamic_cast in the critical path
Earl Ou has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/34355 )
Change subject: systemc: avoid dynamic_cast in the critical path
..
systemc: avoid dynamic_cast in the critical path
NodeList is in the critical path of the systemc scheduler in gem5. A
unnecessary dynamic_cast in the NodeList slow down the event process by
about 15%. Fix the issue by avoiding dynamic_cast.
We see about 15% speed improvement on the example provided by Matthias Jung:
https://gist.github.com/myzinsky/557200aa04556de44a317e0a10f51840
Compare with Accellera implementation, gem5 version is originally 10x
slower and now it's about 8.5x slower.
Change-Id: I3b4ddca31e58e1d4e96144a4021b0a5bb956fda4
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34355
Reviewed-by: Gabe Black
Maintainer: Gabe Black
Tested-by: kokoro
---
M src/systemc/core/list.hh
1 file changed, 7 insertions(+), 2 deletions(-)
Approvals:
Gabe Black: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/systemc/core/list.hh b/src/systemc/core/list.hh
index b1c5f55..6ba2825 100644
--- a/src/systemc/core/list.hh
+++ b/src/systemc/core/list.hh
@@ -102,8 +102,13 @@
prevListNode = t;
}
-T *getNext() { return dynamic_cast(nextListNode); }
-bool empty() { return getNext() == nullptr; }
+T *
+getNext()
+{
+return empty() ? nullptr : static_cast(nextListNode);
+}
+
+bool empty() { return nextListNode == this; }
};
} // namespace sc_gem5
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34355
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: I3b4ddca31e58e1d4e96144a4021b0a5bb956fda4
Gerrit-Change-Number: 34355
Gerrit-PatchSet: 6
Gerrit-Owner: Earl Ou
Gerrit-Reviewer: Earl Ou
Gerrit-Reviewer: Gabe Black
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]: mips,cpu: Get rid of the IsIprAccess StaticInst flag.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/33739 )
Change subject: mips,cpu: Get rid of the IsIprAccess StaticInst flag.
..
mips,cpu: Get rid of the IsIprAccess StaticInst flag.
This was set by MIPS in two places, I think largely just because it was
available. This flag refers to IPRs which are an Alpha concept. In the
O3 CPU, IsIprAccess was used as a possible indicator to determine if an
instruction IsSerializeBefore, but we've already got a flag for that. In
the minor CPU, which hasn't been made to work with MIPS as far as I
know, it was used in a condition but not mentioned in the comment
alongside the condition. I think there it was added for the sake of
Alpha.
This change eliminates that flag and removes it from the O3 and minor
CPUs. In the MIPS ISA description, the instructions that were marked as
IsIprAccess have now been marked as IsSerializeBefore since, if there
was a real reason for them to be marked as IsIprAccess, it would have
been to get it them to work in O3, and there IsSerializeBefore gets
equivalent behavior.
Change-Id: Ia874cde12fa70b998d3e638458f13d69798d40b7
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/33739
Maintainer: Gabe Black
Tested-by: kokoro
Reviewed-by: Jason Lowe-Power
---
M src/arch/mips/isa/decoder.isa
M src/cpu/StaticInstFlags.py
M src/cpu/base_dyn_inst.hh
M src/cpu/minor/execute.cc
M src/cpu/o3/rename_impl.hh
M src/cpu/static_inst.hh
6 files changed, 4 insertions(+), 9 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/mips/isa/decoder.isa b/src/arch/mips/isa/decoder.isa
index 76453b0..73e2b5d 100644
--- a/src/arch/mips/isa/decoder.isa
+++ b/src/arch/mips/isa/decoder.isa
@@ -174,10 +174,10 @@
0x2: decode FUNCTION_LO {
0x0: HiLoRsSelOp::mfhi({{ Rd = HI_RS_SEL; }},
- IntMultOp, IsIprAccess);
+ IntMultOp, IsSerializeBefore);
0x1: HiLoRdSelOp::mthi({{ HI_RD_SEL = Rs; }});
0x2: HiLoRsSelOp::mflo({{ Rd = LO_RS_SEL; }},
- IntMultOp, IsIprAccess);
+ IntMultOp, IsSerializeBefore);
0x3: HiLoRdSelOp::mtlo({{ LO_RD_SEL = Rs; }});
}
diff --git a/src/cpu/StaticInstFlags.py b/src/cpu/StaticInstFlags.py
index b70f919..151074e 100644
--- a/src/cpu/StaticInstFlags.py
+++ b/src/cpu/StaticInstFlags.py
@@ -89,7 +89,6 @@
'IsNonSpeculative', # Should not be executed speculatively
'IsQuiesce',# Is a quiesce instruction
-'IsIprAccess', # Accesses IPRs
'IsUnverifiable', # Can't be verified by a checker
'IsSyscall',# Causes a system call to be emulated in
syscall
diff --git a/src/cpu/base_dyn_inst.hh b/src/cpu/base_dyn_inst.hh
index bfe0492..00639ad 100644
--- a/src/cpu/base_dyn_inst.hh
+++ b/src/cpu/base_dyn_inst.hh
@@ -557,7 +557,6 @@
bool isWriteBarrier() const { return staticInst->isWriteBarrier(); }
bool isNonSpeculative() const { return staticInst->isNonSpeculative();
}
bool isQuiesce() const { return staticInst->isQuiesce(); }
-bool isIprAccess() const { return staticInst->isIprAccess(); }
bool isUnverifiable() const { return staticInst->isUnverifiable(); }
bool isSyscall() const { return staticInst->isSyscall(); }
bool isMacroop() const { return staticInst->isMacroop(); }
diff --git a/src/cpu/minor/execute.cc b/src/cpu/minor/execute.cc
index 45ca002..f8db523 100644
--- a/src/cpu/minor/execute.cc
+++ b/src/cpu/minor/execute.cc
@@ -224,8 +224,7 @@
!inst->isFault() &&
inst->isLastOpInInst() &&
(inst->staticInst->isSerializeAfter() ||
- inst->staticInst->isSquashAfter() ||
- inst->staticInst->isIprAccess());
+ inst->staticInst->isSquashAfter());
DPRINTF(Branch, "tryToBranch before: %s after: %s%s\n",
pc_before, target, (force_branch ? " (forcing)" : ""));
diff --git a/src/cpu/o3/rename_impl.hh b/src/cpu/o3/rename_impl.hh
index 1cbe87a..052012e 100644
--- a/src/cpu/o3/rename_impl.hh
+++ b/src/cpu/o3/rename_impl.hh
@@ -684,8 +684,7 @@
// instructions. This is mainly due to lack of support for
// out-of-order operations of either of those classes of
// instructions.
-if ((inst->isIprAccess() || inst->isSerializeBefore()) &&
-!inst->isSerializeHandled()) {
+if (inst->isSerializeBefore() && !inst->isSerializeHandled()) {
DPRINTF(Rename, "Serialize before instruction encountered.\n");
if (!inst->isTempSerializeBefore()) {
diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh
index e536b84..353c0e3 100644
--- a/src/cpu/static_inst.hh
+++ b/src/cpu/static_inst.hh
@@ -190,7
[gem5-dev] Change in gem5/gem5[develop]: mips,cpu: Get rid of the IsCondDelaySlot StaticInst flag.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/33736 )
Change subject: mips,cpu: Get rid of the IsCondDelaySlot StaticInst flag.
..
mips,cpu: Get rid of the IsCondDelaySlot StaticInst flag.
This is set by MIPS in a few places, but not actually used by anything.
Change-Id: Iaf3b29b2c14bb1de3ffd6a0035f12f238591cb60
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/33736
Maintainer: Gabe Black
Tested-by: kokoro
Reviewed-by: Jason Lowe-Power
---
M src/arch/mips/isa/formats/branch.isa
M src/cpu/StaticInstFlags.py
M src/cpu/base_dyn_inst.hh
M src/cpu/static_inst.hh
4 files changed, 0 insertions(+), 6 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/mips/isa/formats/branch.isa
b/src/arch/mips/isa/formats/branch.isa
index 4975a13..7c2b27c 100644
--- a/src/arch/mips/isa/formats/branch.isa
+++ b/src/arch/mips/isa/formats/branch.isa
@@ -241,7 +241,6 @@
code += 'R31 = NNPC;\n'
elif x == 'Likely':
not_taken_code = 'NNPC = NPC; NPC = PC;'
-inst_flags += ('IsCondDelaySlot', )
else:
inst_flags += (x, )
@@ -280,7 +279,6 @@
code += 'R32 = NNPC;'
elif x == 'Likely':
not_taken_code = 'NNPC = NPC, NPC = PC;'
-inst_flags += ('IsCondDelaySlot', )
else:
inst_flags += (x, )
diff --git a/src/cpu/StaticInstFlags.py b/src/cpu/StaticInstFlags.py
index 1c2b63a..5b8507b 100644
--- a/src/cpu/StaticInstFlags.py
+++ b/src/cpu/StaticInstFlags.py
@@ -78,8 +78,6 @@
'IsCall', # Subroutine call.
'IsReturn', # Subroutine return.
-'IsCondDelaySlot', # Conditional Delay-Slot Instruction
-
'IsThreadSync', # Thread synchronization operation.
'IsSerializing',# Serializes pipeline: won't execute until all
diff --git a/src/cpu/base_dyn_inst.hh b/src/cpu/base_dyn_inst.hh
index a6c08cc..60ca592 100644
--- a/src/cpu/base_dyn_inst.hh
+++ b/src/cpu/base_dyn_inst.hh
@@ -541,7 +541,6 @@
bool isIndirectCtrl() const { return staticInst->isIndirectCtrl(); }
bool isCondCtrl() const { return staticInst->isCondCtrl(); }
bool isUncondCtrl() const { return staticInst->isUncondCtrl(); }
-bool isCondDelaySlot() const { return staticInst->isCondDelaySlot(); }
bool isThreadSync() const { return staticInst->isThreadSync(); }
bool isSerializing() const { return staticInst->isSerializing(); }
bool
diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh
index 146be8c..1a0e42c 100644
--- a/src/cpu/static_inst.hh
+++ b/src/cpu/static_inst.hh
@@ -179,7 +179,6 @@
bool isIndirectCtrl() const { return flags[IsIndirectControl]; }
bool isCondCtrl() const { return flags[IsCondControl]; }
bool isUncondCtrl() const { return flags[IsUncondControl]; }
-bool isCondDelaySlot() const { return flags[IsCondDelaySlot]; }
bool isThreadSync() const { return flags[IsThreadSync]; }
bool isSerializing() const { return flags[IsSerializing] ||
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/33736
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: Iaf3b29b2c14bb1de3ffd6a0035f12f238591cb60
Gerrit-Change-Number: 33736
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Bobby R. Bruce
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]: mips,cpu: Get rid of the IsERET StaticInst flag.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/33738 )
Change subject: mips,cpu: Get rid of the IsERET StaticInst flag.
..
mips,cpu: Get rid of the IsERET StaticInst flag.
This is set by MIPS but doesn't have an accessor in StaticInst, and
isn't used by anything.
Change-Id: Ie28d2df134dcf264bca17c9c66dd32515a240492
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/33738
Maintainer: Gabe Black
Tested-by: kokoro
Reviewed-by: Jason Lowe-Power
---
M src/arch/mips/isa/decoder.isa
M src/cpu/StaticInstFlags.py
2 files changed, 2 insertions(+), 3 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/mips/isa/decoder.isa b/src/arch/mips/isa/decoder.isa
index f62000e..76453b0 100644
--- a/src/arch/mips/isa/decoder.isa
+++ b/src/arch/mips/isa/decoder.isa
@@ -719,7 +719,7 @@
LLFlag = 0;
Status = status;
SRSCtl = srsCtl;
-}}, IsReturn, IsSerializing, IsERET);
+}}, IsReturn, IsSerializing);
0x1F: deret({{
DebugReg debug = Debug;
@@ -732,7 +732,7 @@
// Undefined;
}
Debug = debug;
-}}, IsReturn, IsSerializing, IsERET);
+}}, IsReturn, IsSerializing);
}
format CP0TLB {
0x01: tlbr({{
diff --git a/src/cpu/StaticInstFlags.py b/src/cpu/StaticInstFlags.py
index acaa7bf..b70f919 100644
--- a/src/cpu/StaticInstFlags.py
+++ b/src/cpu/StaticInstFlags.py
@@ -85,7 +85,6 @@
'IsMemBarrier', # Is a memory barrier
'IsWriteBarrier', # Is a write barrier
'IsReadBarrier',# Is a read barrier
-'IsERET', # <- Causes the IFU to stall (MIPS ISA)
'IsNonSpeculative', # Should not be executed speculatively
'IsQuiesce',# Is a quiesce instruction
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/33738
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: Ie28d2df134dcf264bca17c9c66dd32515a240492
Gerrit-Change-Number: 33738
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Bobby R. Bruce
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]: cpu: Get rid of the IsThreadSync StaticInst flag.
Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/33737 )
Change subject: cpu: Get rid of the IsThreadSync StaticInst flag.
..
cpu: Get rid of the IsThreadSync StaticInst flag.
This flag was never set and only checked in one place. If it was set, it
would have triggered a panic there.
Change-Id: I934a0346837c66bae8ce06f50027003bfd47083d
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/33737
Reviewed-by: Jason Lowe-Power
Reviewed-by: Andreas Sandberg
Maintainer: Andreas Sandberg
Tested-by: kokoro
---
M src/cpu/StaticInstFlags.py
M src/cpu/base_dyn_inst.hh
M src/cpu/o3/commit_impl.hh
M src/cpu/static_inst.hh
4 files changed, 0 insertions(+), 9 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/cpu/StaticInstFlags.py b/src/cpu/StaticInstFlags.py
index 5b8507b..acaa7bf 100644
--- a/src/cpu/StaticInstFlags.py
+++ b/src/cpu/StaticInstFlags.py
@@ -78,8 +78,6 @@
'IsCall', # Subroutine call.
'IsReturn', # Subroutine return.
-'IsThreadSync', # Thread synchronization operation.
-
'IsSerializing',# Serializes pipeline: won't execute until all
# older instructions have committed.
'IsSerializeBefore',
diff --git a/src/cpu/base_dyn_inst.hh b/src/cpu/base_dyn_inst.hh
index 60ca592..bfe0492 100644
--- a/src/cpu/base_dyn_inst.hh
+++ b/src/cpu/base_dyn_inst.hh
@@ -541,7 +541,6 @@
bool isIndirectCtrl() const { return staticInst->isIndirectCtrl(); }
bool isCondCtrl() const { return staticInst->isCondCtrl(); }
bool isUncondCtrl() const { return staticInst->isUncondCtrl(); }
-bool isThreadSync() const { return staticInst->isThreadSync(); }
bool isSerializing() const { return staticInst->isSerializing(); }
bool
isSerializeBefore() const
diff --git a/src/cpu/o3/commit_impl.hh b/src/cpu/o3/commit_impl.hh
index 75d065f..0d5cbe5 100644
--- a/src/cpu/o3/commit_impl.hh
+++ b/src/cpu/o3/commit_impl.hh
@@ -1233,11 +1233,6 @@
return false;
}
-if (head_inst->isThreadSync()) {
-// Not handled for now.
-panic("Thread sync instructions are not handled yet.\n");
-}
-
// Check if the instruction caused a fault. If so, trap.
Fault inst_fault = head_inst->getFault();
diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh
index 1a0e42c..e536b84 100644
--- a/src/cpu/static_inst.hh
+++ b/src/cpu/static_inst.hh
@@ -180,7 +180,6 @@
bool isCondCtrl() const { return flags[IsCondControl]; }
bool isUncondCtrl() const { return flags[IsUncondControl]; }
-bool isThreadSync() const { return flags[IsThreadSync]; }
bool isSerializing() const { return flags[IsSerializing] ||
flags[IsSerializeBefore] ||
flags[IsSerializeAfter]; }
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/33737
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: I934a0346837c66bae8ce06f50027003bfd47083d
Gerrit-Change-Number: 33737
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Giacomo Travaglini
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: kokoro
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
[gem5-dev] Re: MessageBuffer double counting delay between arrival and dequeue ticks?
Hi Ryan, You are right. If the message goes to the next message buffer, delay is added again. However, as far as I know we are not using the delayed ticks for calculating anything anymore. The earlier use case was to calculate the stall time within MessageBuffer, but we use the getTime to do that now. Is there any specific use case you are trying to fix here? In any case, it would be great if you can post a fix. Thanks, Srikant On Mon, Sep 14, 2020 at 10:48 AM Jason Lowe-Power via gem5-dev < gem5-dev@gem5.org> wrote: > Hey Ryan, > > Sorry for the slow reply. Yes, it looks like delayedTicks may be double > counting in some cases. I wonder if a little microbenchmark might be able > to confirm more clearly? Assuming it is being double counted, we'd welcome > a fix! > > @Bharadwaj, Srikant might be able to help as > well. Srikant, did you see anything like this with Garnet 3.0? > > Cheers, > Jason > > On Fri, Sep 11, 2020 at 2:35 AM Gambord, Ryan via gem5-dev < > gem5-dev@gem5.org> wrote: > >> bump >> >> On Tue, Sep 1, 2020, 12:21 Gambord, Ryan >> wrote: >> >>> I noticed that MessageBuffer calls UpdateDelayedTicks in both enqueue >>> and dequeue methods. Since dequeue does not setLastEnqueueTime of the >>> message to the time it was dequeued at, when enqueue calls >>> UpdateDelayedTicks, doesn't it add the dequeue delay to the delayed ticks a >>> second time? >>> >>> Below is a table of the timeline. X and Y are the starting values for >>> LastEnqueueTime and DelayedTicks when the first message buffer receives the >>> message. When the message is dequeued from MB1, DelayedTicks gets C-B added >>> to it. When it is then enqueued in MB2, it gets D-B added, which double >>> counts the C-B interval. >>> >>> curTime() FunctionCall m_LastEnqueueTime m_DelayedTicks >>> >>> >>> X Y >>> A enqueue() B = A + Delta Y + (A-X) >>> B wakeup() " " >>> C dequeue() " Y + (A-X) + (C-B) >>> D enqueue() E = D + Delta >>> Y + (A-X) + (C-B) + (D-B) = Y + (A-X) + (C-B) + (D-C) + (C-B) >>> >>> >>> Ryan Gambord >>> >>> >>> ___ >> gem5-dev mailing list -- gem5-dev@gem5.org >> To unsubscribe send an email to gem5-dev-le...@gem5.org >> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s > > ___ > gem5-dev mailing list -- gem5-dev@gem5.org > To unsubscribe send an email to gem5-dev-le...@gem5.org > %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s ___ gem5-dev mailing list -- gem5-dev@gem5.org To unsubscribe send an email to gem5-dev-le...@gem5.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
