Boris Shingarov has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/40947 )
Change subject: arch-power: Fix load-store timing sequence
......................................................................
arch-power: Fix load-store timing sequence
To properly implement load-store instructions for use with
the TimingSimpleCPU model, the initiateAcc() part of the
instruction should only be responsible for performing the
effective address computation and then initiating memory
access.
The completeAcc() part of the instruction should then be
responsible for setting the condition register flags or
updating the base register based on the outcome of the
memory access. This fixes the following instructions:
* Load Byte and Zero with Update (lbzu)
* Load Halfword and Zero with Update (lhzu)
* Load Halfword Algebraic with Update (lhau)
* Load Word and Zero with Update (lwzu)
* Load Doubleword with Update (ldu)
* Load Floating Single with Update (lfsu)
* Load Floating Double with Update (lfdu)
* Load Byte and Zero with Update Indexed (lbzux)
* Load Halfword and Zero with Update Indexed (lhzux)
* Load Halfword Algebraic with Update Indexed (lhaux)
* Load Word and Zero with Update Indexed (lwzux)
* Load Word Algebraic with Update Indexed (lwaux)
* Load Doubleword with Update Indexed (ldux)
* Load Floating Single with Update Indexed (lfsux)
* Load Floating Double with Update Indexed (lfdux)
* Load Byte And Reserve Indexed (lbarx)
* Load Halfword And Reserve Indexed (lharx)
* Load Word And Reserve Indexed (lwarx)
* Load Doubleword And Reserve Indexed (ldarx)
* Store Byte with Update (stbu)
* Store Halfword with Update (sthu)
* Store Word with Update (stwu)
* Store Doubleword with Update (stdu)
* Store Byte with Update Indexed (stbux)
* Store Halfword with Update Indexed (sthux)
* Store Word with Update Indexed (stwux)
* Store Doubleword with Update Indexed (stdux)
* Store Byte Conditional Indexed (stbcx.)
* Store Halfword Conditional Indexed (sthcx.)
* Store Word Conditional Indexed (stwcx.)
* Store Doubleword Conditional Indexed (stdcx.)
* Store Floating Single with Update (stfsu)
* Store Floating Double with Update (stdsu)
* Store Floating Single with Update Indexed (stfsux)
* Store Floating Double with Update Indexed (stfdux)
Change-Id: If5f720619ec3c40a90c1362a9dfc8cc204e57acf
Signed-off-by: Sandipan Das <[email protected]>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40947
Reviewed-by: Boris Shingarov <[email protected]>
Maintainer: Boris Shingarov <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/arch/power/isa/decoder.isa
M src/arch/power/isa/formats/mem.isa
M src/arch/power/isa/formats/util.isa
3 files changed, 73 insertions(+), 55 deletions(-)
Approvals:
Boris Shingarov: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/power/isa/decoder.isa b/src/arch/power/isa/decoder.isa
index 461e2cb..a40fbca 100644
--- a/src/arch/power/isa/decoder.isa
+++ b/src/arch/power/isa/decoder.isa
@@ -333,10 +333,8 @@
4: IntTrapOp::tw({{ Ra_sw }}, {{ Rb_sw }});
format LoadIndexOp {
- 20: lwarx({{
- Rt = Mem_uw;
- Rsv = 1; RsvLen = 4; RsvAddr = EA;
- }});
+ 20: lwarx({{ Rt = Mem_uw; }},
+ {{ Rsv = 1; RsvLen = 4; RsvAddr = EA; }});
21: ldx({{ Rt = Mem; }});
23: lwzx({{ Rt = Mem_uw; }});
@@ -373,10 +371,8 @@
cr = makeCRFieldUnsigned((uint32_t)Ra, (uint32_t)Rb,
xer.so);
}});
- 52: LoadIndexOp::lbarx({{
- Rt = Mem_ub;
- Rsv = 1; RsvLen = 1; RsvAddr = EA;
- }});
+ 52: LoadIndexOp::lbarx({{ Rt = Mem_ub; }},
+ {{ Rsv = 1; RsvLen = 1; RsvAddr = EA; }});
53: LoadIndexUpdateOp::ldux({{ Rt = Mem; }});
55: LoadIndexUpdateOp::lwzux({{ Rt = Mem_uw; }});
@@ -389,17 +385,13 @@
68: IntTrapOp::td({{ Ra }}, {{ Rb }});
format LoadIndexOp {
- 84: ldarx({{
- Rt = Mem_ud;
- Rsv = 1; RsvLen = 8; RsvAddr = EA;
- }});
+ 84: ldarx({{ Rt = Mem_ud; }},
+ {{ Rsv = 1; RsvLen = 8; RsvAddr = EA; }});
87: lbzx({{ Rt = Mem_ub; }});
- 116: lharx({{
- Rt = Mem_uh;
- Rsv = 1; RsvLen = 2; RsvAddr = EA;
- }});
+ 116: lharx({{ Rt = Mem_uh;}},
+ {{ Rsv = 1; RsvLen = 2; RsvAddr = EA; }});
}
119: LoadIndexUpdateOp::lbzux({{ Rt = Mem_ub; }});
@@ -424,8 +416,9 @@
format StoreIndexOp {
149: stdx({{ Mem = Rs }});
150: stwcx({{
- bool store_performed = false;
Mem_uw = Rs_uw;
+ }}, {{
+ bool store_performed = false;
if (Rsv) {
if (RsvLen == 4) {
if (RsvAddr == EA) {
@@ -481,8 +474,9 @@
format StoreIndexOp {
214: stdcx({{
- bool store_performed = false;
Mem = Rs;
+ }}, {{
+ bool store_performed = false;
if (Rsv) {
if (RsvLen == 8) {
if (RsvAddr == EA) {
@@ -656,8 +650,9 @@
663: stfsx({{ Mem_sf = Fs_sf; }});
694: stbcx({{
- bool store_performed = false;
Mem_ub = Rs_ub;
+ }}, {{
+ bool store_performed = false;
if (Rsv) {
if (RsvLen == 1) {
if (RsvAddr == EA) {
@@ -677,8 +672,9 @@
format StoreIndexOp {
726: sthcx({{
- bool store_performed = false;
Mem_uh = Rs_uh;
+ }}, {{
+ bool store_performed = false;
if (Rsv) {
if (RsvLen == 2) {
if (RsvAddr == EA) {
diff --git a/src/arch/power/isa/formats/mem.isa
b/src/arch/power/isa/formats/mem.isa
index d499903..cdd8729 100644
--- a/src/arch/power/isa/formats/mem.isa
+++ b/src/arch/power/isa/formats/mem.isa
@@ -82,6 +82,7 @@
}
if (fault == NoFault) {
+ %(update_code)s;
%(op_wb)s;
}
@@ -120,9 +121,8 @@
Msr msr = xc->tcBase()->readIntReg(INTREG_MSR);
%(op_decl)s;
- %(op_rd)s;
-
EA = pkt->req->getVaddr();
+ %(op_rd)s;
if (msr.le)
getMemLE(pkt, Mem, traceData);
@@ -134,7 +134,8 @@
}
if (fault == NoFault) {
- %(op_wb)s;
+ %(update_code)s;
+ %(op_wb)s;
}
return fault;
@@ -167,6 +168,7 @@
}
if (fault == NoFault) {
+ %(update_code)s;
%(op_wb)s;
}
@@ -199,11 +201,6 @@
NULL);
}
- // Need to write back any potential address register update
- if (fault == NoFault) {
- %(op_wb)s;
- }
-
return fault;
}
}};
@@ -213,7 +210,20 @@
Fault %(class_name)s::completeAcc(PacketPtr pkt, ExecContext *xc,
Trace::InstRecord *traceData) const
{
- return NoFault;
+ GEM5_VAR_USED Addr EA;
+ Fault fault = NoFault;
+
+ %(op_decl)s;
+ EA = pkt->req->getVaddr();
+ %(op_rd)s;
+
+ // Need to write back any potential address register update
+ if (fault == NoFault) {
+ %(update_code)s;
+ %(op_wb)s;
+ }
+
+ return fault;
}
}};
@@ -224,19 +234,22 @@
// dependence on Ra.
let {{
-def GenMemOp(name, Name, memacc_code, ea_code, ea_code_ra0, base,
+def GenMemOp(name, Name, memacc_code, update_code, ea_code, ea_code_ra0,
base,
load_or_store, mem_flags = [], inst_flags = []):
# First the version where Ra is non-zero
(header_output, decoder_output, decode_block, exec_output) = \
- LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags,
inst_flags,
+ LoadStoreBase(name, Name, ea_code,
+ memacc_code, update_code,
+ mem_flags, inst_flags,
base_class = base,
decode_template = CheckRaDecode,
exec_template_base = load_or_store)
# Now another version where Ra == 0
(header_output_ra0, decoder_output_ra0, _, exec_output_ra0) = \
- LoadStoreBase(name, Name + 'RaZero', ea_code_ra0, memacc_code,
+ LoadStoreBase(name, Name + 'RaZero', ea_code_ra0,
+ memacc_code, update_code,
mem_flags, inst_flags,
base_class = base,
exec_template_base = load_or_store)
@@ -250,20 +263,22 @@
}};
-def format LoadIndexOp(memacc_code, ea_code = {{ EA = Ra + Rb; }},
+def format LoadIndexOp(memacc_code, update_code = {{ }},
+ ea_code = {{ EA = Ra + Rb; }},
ea_code_ra0 = {{ EA = Rb; }},
mem_flags = [], inst_flags = []) {{
(header_output, decoder_output, decode_block, exec_output) = \
- GenMemOp(name, Name, memacc_code, ea_code, ea_code_ra0,
+ GenMemOp(name, Name, memacc_code, update_code, ea_code,
ea_code_ra0,
'MemIndexOp', 'Load', mem_flags, inst_flags)
}};
-def format StoreIndexOp(memacc_code, ea_code = {{ EA = Ra + Rb; }},
+def format StoreIndexOp(memacc_code, update_code = {{ }},
+ ea_code = {{ EA = Ra + Rb; }},
ea_code_ra0 = {{ EA = Rb; }},
mem_flags = [], inst_flags = []) {{
(header_output, decoder_output, decode_block, exec_output) = \
- GenMemOp(name, Name, memacc_code, ea_code, ea_code_ra0,
+ GenMemOp(name, Name, memacc_code, update_code, ea_code,
ea_code_ra0,
'MemIndexOp', 'Store', mem_flags, inst_flags)
}};
@@ -272,11 +287,12 @@
mem_flags = [], inst_flags = []) {{
# Add in the update code
- memacc_code += 'Ra = EA;'
+ update_code = 'Ra = EA;'
# Generate the class
(header_output, decoder_output, decode_block, exec_output) = \
- LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags,
inst_flags,
+ LoadStoreBase(name, Name, ea_code, memacc_code, update_code,
+ mem_flags, inst_flags,
base_class = 'MemIndexOp',
decode_template = CheckRaRtDecode,
exec_template_base = 'Load')
@@ -287,11 +303,12 @@
mem_flags = [], inst_flags = []) {{
# Add in the update code
- memacc_code += 'Ra = EA;'
+ update_code = 'Ra = EA;'
# Generate the class
(header_output, decoder_output, decode_block, exec_output) = \
- LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags,
inst_flags,
+ LoadStoreBase(name, Name, ea_code, memacc_code, update_code,
+ mem_flags, inst_flags,
base_class = 'MemIndexOp',
decode_template = CheckRaZeroDecode,
exec_template_base = 'Store')
@@ -302,7 +319,7 @@
ea_code_ra0 = {{ EA = d; }},
mem_flags = [], inst_flags = []) {{
(header_output, decoder_output, decode_block, exec_output) = \
- GenMemOp(name, Name, memacc_code, ea_code, ea_code_ra0,
+ GenMemOp(name, Name, memacc_code, '', ea_code, ea_code_ra0,
'MemDispOp', 'Load', mem_flags, inst_flags)
}};
@@ -311,7 +328,7 @@
ea_code_ra0 = {{ EA = d; }},
mem_flags = [], inst_flags = []) {{
(header_output, decoder_output, decode_block, exec_output) = \
- GenMemOp(name, Name, memacc_code, ea_code, ea_code_ra0,
+ GenMemOp(name, Name, memacc_code, '', ea_code, ea_code_ra0,
'MemDispOp', 'Store', mem_flags, inst_flags)
}};
@@ -321,7 +338,7 @@
ea_code_ra0 = {{ EA = (ds << 2); }},
mem_flags = [], inst_flags = []) {{
(header_output, decoder_output, decode_block, exec_output) = \
- GenMemOp(name, Name, memacc_code, ea_code, ea_code_ra0,
+ GenMemOp(name, Name, memacc_code, '', ea_code, ea_code_ra0,
'MemDispShiftOp', 'Load', mem_flags, inst_flags)
}};
@@ -331,7 +348,7 @@
ea_code_ra0 = {{ EA = (ds << 2); }},
mem_flags = [], inst_flags = []) {{
(header_output, decoder_output, decode_block, exec_output) = \
- GenMemOp(name, Name, memacc_code, ea_code, ea_code_ra0,
+ GenMemOp(name, Name, memacc_code, '', ea_code, ea_code_ra0,
'MemDispShiftOp', 'Store', mem_flags, inst_flags)
}};
@@ -340,11 +357,12 @@
mem_flags = [], inst_flags = []) {{
# Add in the update code
- memacc_code += 'Ra = EA;'
+ update_code = 'Ra = EA;'
# Generate the class
(header_output, decoder_output, decode_block, exec_output) = \
- LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags,
inst_flags,
+ LoadStoreBase(name, Name, ea_code, memacc_code, update_code,
+ mem_flags, inst_flags,
base_class = 'MemDispOp',
decode_template = CheckRaRtDecode,
exec_template_base = 'Load')
@@ -355,11 +373,12 @@
mem_flags = [], inst_flags = []) {{
# Add in the update code
- memacc_code += 'Ra = EA;'
+ update_code = 'Ra = EA;'
# Generate the class
(header_output, decoder_output, decode_block, exec_output) = \
- LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags,
inst_flags,
+ LoadStoreBase(name, Name, ea_code, memacc_code, update_code,
+ mem_flags, inst_flags,
base_class = 'MemDispOp',
decode_template = CheckRaZeroDecode,
exec_template_base = 'Store')
@@ -371,11 +390,12 @@
mem_flags = [], inst_flags = []) {{
# Add in the update code
- memacc_code += 'Ra = EA;'
+ update_code = 'Ra = EA;'
# Generate the class
(header_output, decoder_output, decode_block, exec_output) = \
- LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags,
inst_flags,
+ LoadStoreBase(name, Name, ea_code, memacc_code, update_code,
+ mem_flags, inst_flags,
base_class = 'MemDispShiftOp',
decode_template = CheckRaRtDecode,
exec_template_base = 'Load')
@@ -387,11 +407,12 @@
mem_flags = [], inst_flags = []) {{
# Add in the update code
- memacc_code += 'Ra = EA;'
+ update_code = 'Ra = EA;'
# Generate the class
(header_output, decoder_output, decode_block, exec_output) = \
- LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags,
inst_flags,
+ LoadStoreBase(name, Name, ea_code, memacc_code, update_code,
+ mem_flags, inst_flags,
base_class = 'MemDispShiftOp',
decode_template = CheckRaZeroDecode,
exec_template_base = 'Store')
diff --git a/src/arch/power/isa/formats/util.isa
b/src/arch/power/isa/formats/util.isa
index dbc6150..00132b3 100644
--- a/src/arch/power/isa/formats/util.isa
+++ b/src/arch/power/isa/formats/util.isa
@@ -145,8 +145,8 @@
let {{
-def LoadStoreBase(name, Name, ea_code, memacc_code, mem_flags, inst_flags,
- base_class = 'MemOp',
+def LoadStoreBase(name, Name, ea_code, memacc_code, update_code,
+ mem_flags, inst_flags, base_class = 'MemOp',
decode_template = BasicDecode, exec_template_base = ''):
# Make sure flags are in lists (convert to lists if not).
mem_flags = makeList(mem_flags)
@@ -155,7 +155,8 @@
# Generate InstObjParams for the memory access.
iop = InstObjParams(name, Name, base_class,
{'ea_code': ea_code,
- 'memacc_code': memacc_code},
+ 'memacc_code': memacc_code,
+ 'update_code': update_code},
inst_flags)
if mem_flags:
11 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the
submitted one.
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/40947
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: If5f720619ec3c40a90c1362a9dfc8cc204e57acf
Gerrit-Change-Number: 40947
Gerrit-PatchSet: 13
Gerrit-Owner: Sandipan Das <[email protected]>
Gerrit-Reviewer: Boris Shingarov <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
