gem5-dev@gem5.org

The gem5 Developer List

View all threads

[M] Change in gem5/gem5[develop]: cpu: Merge TimingExprSrcReg and TimingExprReadIntReg.

GB
Gabe Black (Gerrit)
Fri, Jun 24, 2022 11:26 AM

Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/49776 )

Change subject: cpu: Merge TimingExprSrcReg and TimingExprReadIntReg.
......................................................................

cpu: Merge TimingExprSrcReg and TimingExprReadIntReg.

Make it possible to read any type of reg, assuming it fits in a RegVal.
This avoids assuming building in a dependency on the readIntReg
accessor.

It also avoids setting up a situation where the API could at least
theoretically base the timing expression on the value of any int reg,
even ones the instruction does not interact with. The ...ReadIntReg
expression was only ever used with the result of the ...SrcReg
expression, and in my opinion, that's realy the only way it makes sense
to use it. It doesn't seem useful to split that operation into two
parts.

If it actually does make sense (although I doubt this), these operations
can't really be generalized easily since the TimingExpr... classes all
expect to pass around uint64_ts, and a RegId, the real value of a
SrcReg index which does not assume a register type, would not fit in
that in the general case.

Change-Id: I253a0a058dc078deeb28ef0babead4c8ffc3b792
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49776
Tested-by: kokoro noreply+kokoro@google.com
Reviewed-by: Giacomo Travaglini giacomo.travaglini@arm.com
Maintainer: Gabe Black gabe.black@gmail.com

M configs/common/cores/arm/HPI.py
M src/cpu/SConscript
M src/cpu/TimingExpr.py
M src/cpu/timing_expr.cc
M src/cpu/timing_expr.hh
5 files changed, 43 insertions(+), 53 deletions(-)

Approvals:
kokoro: Regressions pass
Giacomo Travaglini: Looks good to me, approved
Gabe Black: Looks good to me, approved

diff --git a/configs/common/cores/arm/HPI.py
b/configs/common/cores/arm/HPI.py
index 3a11133..8f3a113 100644
--- a/configs/common/cores/arm/HPI.py
+++ b/configs/common/cores/arm/HPI.py
@@ -144,20 +144,13 @@
return ret
return body

-def src(index):
+def src_reg(index):
def body(env):
ret = TimingExprSrcReg()
ret.index = index
return ret
return body

-def int_reg(reg):

  • def body(env):

  •    ret = TimingExprReadIntReg()
    
  •    ret.reg = reg(env)
    
  •    return ret
    
  • return body

  • def let(bindings, expr):
    def body(env):
    ret = TimingExprLet()
    @@ -488,8 +481,8 @@

    SDIV

    sdiv_lat_expr = expr_top(let([

  • ('left', un('SignExtend32To64', int_reg(src(4)))),

  • ('right', un('SignExtend32To64', int_reg(src(3)))),

  • ('left', un('SignExtend32To64', src_reg(4))),
  • ('right', un('SignExtend32To64', src_reg(3))),
    ('either_signed', bin('Or',
    bin('SLessThan', ref('left'), literal(0)),
    bin('SLessThan', ref('right'), literal(0)))),
    @@ -511,8 +504,8 @@
    ))

sdiv_lat_expr64 = expr_top(let([

  • ('left', un('SignExtend32To64', int_reg(src(0)))),
  • ('right', un('SignExtend32To64', int_reg(src(1)))),
  • ('left', un('SignExtend32To64', src_reg(0))),
  • ('right', un('SignExtend32To64', src_reg(1))),
    ('either_signed', bin('Or',
    bin('SLessThan', ref('left'), literal(0)),
    bin('SLessThan', ref('right'), literal(0)))),
    @@ -773,8 +766,8 @@
    mask, match = t32_opcode('1111_1011_1011_xxxx__xxxx_xxxx_1111_xxxx')

udiv_lat_expr = expr_top(let([

  • ('left', int_reg(src(4))),
  • ('right', int_reg(src(3))),
  • ('left', src_reg(4)),
  • ('right', src_reg(3)),
    ('left_size', un('SizeInBits', ref('left'))),
    ('right_size', un('SizeInBits',
    bin('UDiv', ref('right'), literal(2)))),
    diff --git a/src/cpu/SConscript b/src/cpu/SConscript
    index fad601e..446dc94 100644
    --- a/src/cpu/SConscript
    +++ b/src/cpu/SConscript
    @@ -98,9 +98,8 @@
    SimObject('CPUTracers.py', sim_objects=[
    'ExeTracer', 'IntelTrace', 'NativeTrace'])
    SimObject('TimingExpr.py', sim_objects=[
  • 'TimingExpr', 'TimingExprLiteral', 'TimingExprSrcReg',
  • 'TimingExprReadIntReg', 'TimingExprLet', 'TimingExprRef', 'TimingExprUn',
  • 'TimingExprBin', 'TimingExprIf'],
  • 'TimingExpr', 'TimingExprLiteral', 'TimingExprSrcReg', 'TimingExprLet',
  • 'TimingExprRef', 'TimingExprUn', 'TimingExprBin', 'TimingExprIf'],
    enums=['TimingExprOp'])

Source('activity.cc')
diff --git a/src/cpu/TimingExpr.py b/src/cpu/TimingExpr.py
index 9c45097..2a6c49a 100644
--- a/src/cpu/TimingExpr.py
+++ b/src/cpu/TimingExpr.py
@@ -66,30 +66,17 @@
value = 0

class TimingExprSrcReg(TimingExpr):

  • """Find the source register number from the current inst"""
  • """Read a source register from the current inst"""
    type = 'TimingExprSrcReg'
    cxx_header = 'cpu/timing_expr.hh'
    cxx_class = 'gem5::TimingExprSrcReg'
  • index = Param.Unsigned("index into inst src regs")

  • index = Param.Unsigned("index into inst src regs")
  • index = Param.Unsigned("index into inst src regs of the reg to read")

    def set_params(self, index):
    self.index = index
    return self

-class TimingExprReadIntReg(TimingExpr):

  • """Read an architectural register"""
  • type = 'TimingExprReadIntReg'
  • cxx_header = 'cpu/timing_expr.hh'
  • cxx_class = 'gem5::TimingExprReadIntReg'
  • reg = Param.TimingExpr("register raw index to read")
  • def set_params(self, reg):
  •    self.reg = reg
    
  •    return self
    
  • class TimingExprLet(TimingExpr):
    """Block of declarations"""
    type = 'TimingExprLet'
    diff --git a/src/cpu/timing_expr.cc b/src/cpu/timing_expr.cc
    index 41868a5..d1f8186 100644
    --- a/src/cpu/timing_expr.cc
    +++ b/src/cpu/timing_expr.cc
    @@ -59,13 +59,7 @@
    uint64_t
    TimingExprSrcReg::eval(TimingExprEvalContext &context)
    {
  • return context.inst->srcRegIdx(index).index();
    -}

-uint64_t
-TimingExprReadIntReg::eval(TimingExprEvalContext &context)
-{

  • return context.thread->readIntReg(reg->eval(context));
  • return context.thread->getReg(context.inst->srcRegIdx(index));
    }

uint64_t
diff --git a/src/cpu/timing_expr.hh b/src/cpu/timing_expr.hh
index 170364e..76212bd 100644
--- a/src/cpu/timing_expr.hh
+++ b/src/cpu/timing_expr.hh
@@ -55,7 +55,6 @@
#include "params/TimingExprIf.hh"
#include "params/TimingExprLet.hh"
#include "params/TimingExprLiteral.hh"
-#include "params/TimingExprReadIntReg.hh"
#include "params/TimingExprRef.hh"
#include "params/TimingExprSrcReg.hh"
#include "params/TimingExprUn.hh"
@@ -124,19 +123,6 @@
uint64_t eval(TimingExprEvalContext &context);
};

-class TimingExprReadIntReg : public TimingExpr
-{

  • public:
  • TimingExpr *reg;
  • TimingExprReadIntReg(const TimingExprReadIntRegParams &params) :
  •    TimingExpr(params),
    
  •    reg(params.reg)
    
  • { }
  • uint64_t eval(TimingExprEvalContext &context);
    -};
  • class TimingExprLet : public TimingExpr
    {
    public:

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49776
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: I253a0a058dc078deeb28ef0babead4c8ffc3b792
Gerrit-Change-Number: 49776
Gerrit-PatchSet: 77
Gerrit-Owner: Gabe Black gabe.black@gmail.com
Gerrit-Reviewer: Gabe Black gabe.black@gmail.com
Gerrit-Reviewer: Giacomo Travaglini giacomo.travaglini@arm.com
Gerrit-Reviewer: Jason Lowe-Power jason@lowepower.com
Gerrit-Reviewer: kokoro noreply+kokoro@google.com
Gerrit-MessageType: merged

Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/49776 ) Change subject: cpu: Merge TimingExprSrcReg and TimingExprReadIntReg. ...................................................................... cpu: Merge TimingExprSrcReg and TimingExprReadIntReg. Make it possible to read any type of reg, assuming it fits in a RegVal. This avoids assuming building in a dependency on the readIntReg accessor. It also avoids setting up a situation where the API could at least theoretically base the timing expression on the value of *any* int reg, even ones the instruction does not interact with. The ...ReadIntReg expression was only ever used with the result of the ...SrcReg expression, and in my opinion, that's realy the only way it makes sense to use it. It doesn't seem useful to split that operation into two parts. If it actually does make sense (although I doubt this), these operations can't really be generalized easily since the TimingExpr... classes all expect to pass around uint64_ts, and a RegId, the *real* value of a SrcReg index which does not assume a register type, would not fit in that in the general case. Change-Id: I253a0a058dc078deeb28ef0babead4c8ffc3b792 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/49776 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Giacomo Travaglini <giacomo.travaglini@arm.com> Maintainer: Gabe Black <gabe.black@gmail.com> --- M configs/common/cores/arm/HPI.py M src/cpu/SConscript M src/cpu/TimingExpr.py M src/cpu/timing_expr.cc M src/cpu/timing_expr.hh 5 files changed, 43 insertions(+), 53 deletions(-) Approvals: kokoro: Regressions pass Giacomo Travaglini: Looks good to me, approved Gabe Black: Looks good to me, approved diff --git a/configs/common/cores/arm/HPI.py b/configs/common/cores/arm/HPI.py index 3a11133..8f3a113 100644 --- a/configs/common/cores/arm/HPI.py +++ b/configs/common/cores/arm/HPI.py @@ -144,20 +144,13 @@ return ret return body -def src(index): +def src_reg(index): def body(env): ret = TimingExprSrcReg() ret.index = index return ret return body -def int_reg(reg): - def body(env): - ret = TimingExprReadIntReg() - ret.reg = reg(env) - return ret - return body - def let(bindings, expr): def body(env): ret = TimingExprLet() @@ -488,8 +481,8 @@ ### SDIV sdiv_lat_expr = expr_top(let([ - ('left', un('SignExtend32To64', int_reg(src(4)))), - ('right', un('SignExtend32To64', int_reg(src(3)))), + ('left', un('SignExtend32To64', src_reg(4))), + ('right', un('SignExtend32To64', src_reg(3))), ('either_signed', bin('Or', bin('SLessThan', ref('left'), literal(0)), bin('SLessThan', ref('right'), literal(0)))), @@ -511,8 +504,8 @@ )) sdiv_lat_expr64 = expr_top(let([ - ('left', un('SignExtend32To64', int_reg(src(0)))), - ('right', un('SignExtend32To64', int_reg(src(1)))), + ('left', un('SignExtend32To64', src_reg(0))), + ('right', un('SignExtend32To64', src_reg(1))), ('either_signed', bin('Or', bin('SLessThan', ref('left'), literal(0)), bin('SLessThan', ref('right'), literal(0)))), @@ -773,8 +766,8 @@ mask, match = t32_opcode('1111_1011_1011_xxxx__xxxx_xxxx_1111_xxxx') udiv_lat_expr = expr_top(let([ - ('left', int_reg(src(4))), - ('right', int_reg(src(3))), + ('left', src_reg(4)), + ('right', src_reg(3)), ('left_size', un('SizeInBits', ref('left'))), ('right_size', un('SizeInBits', bin('UDiv', ref('right'), literal(2)))), diff --git a/src/cpu/SConscript b/src/cpu/SConscript index fad601e..446dc94 100644 --- a/src/cpu/SConscript +++ b/src/cpu/SConscript @@ -98,9 +98,8 @@ SimObject('CPUTracers.py', sim_objects=[ 'ExeTracer', 'IntelTrace', 'NativeTrace']) SimObject('TimingExpr.py', sim_objects=[ - 'TimingExpr', 'TimingExprLiteral', 'TimingExprSrcReg', - 'TimingExprReadIntReg', 'TimingExprLet', 'TimingExprRef', 'TimingExprUn', - 'TimingExprBin', 'TimingExprIf'], + 'TimingExpr', 'TimingExprLiteral', 'TimingExprSrcReg', 'TimingExprLet', + 'TimingExprRef', 'TimingExprUn', 'TimingExprBin', 'TimingExprIf'], enums=['TimingExprOp']) Source('activity.cc') diff --git a/src/cpu/TimingExpr.py b/src/cpu/TimingExpr.py index 9c45097..2a6c49a 100644 --- a/src/cpu/TimingExpr.py +++ b/src/cpu/TimingExpr.py @@ -66,30 +66,17 @@ value = 0 class TimingExprSrcReg(TimingExpr): - """Find the source register number from the current inst""" + """Read a source register from the current inst""" type = 'TimingExprSrcReg' cxx_header = 'cpu/timing_expr.hh' cxx_class = 'gem5::TimingExprSrcReg' - # index = Param.Unsigned("index into inst src regs") - index = Param.Unsigned("index into inst src regs") + index = Param.Unsigned("index into inst src regs of the reg to read") def set_params(self, index): self.index = index return self -class TimingExprReadIntReg(TimingExpr): - """Read an architectural register""" - type = 'TimingExprReadIntReg' - cxx_header = 'cpu/timing_expr.hh' - cxx_class = 'gem5::TimingExprReadIntReg' - - reg = Param.TimingExpr("register raw index to read") - - def set_params(self, reg): - self.reg = reg - return self - class TimingExprLet(TimingExpr): """Block of declarations""" type = 'TimingExprLet' diff --git a/src/cpu/timing_expr.cc b/src/cpu/timing_expr.cc index 41868a5..d1f8186 100644 --- a/src/cpu/timing_expr.cc +++ b/src/cpu/timing_expr.cc @@ -59,13 +59,7 @@ uint64_t TimingExprSrcReg::eval(TimingExprEvalContext &context) { - return context.inst->srcRegIdx(index).index(); -} - -uint64_t -TimingExprReadIntReg::eval(TimingExprEvalContext &context) -{ - return context.thread->readIntReg(reg->eval(context)); + return context.thread->getReg(context.inst->srcRegIdx(index)); } uint64_t diff --git a/src/cpu/timing_expr.hh b/src/cpu/timing_expr.hh index 170364e..76212bd 100644 --- a/src/cpu/timing_expr.hh +++ b/src/cpu/timing_expr.hh @@ -55,7 +55,6 @@ #include "params/TimingExprIf.hh" #include "params/TimingExprLet.hh" #include "params/TimingExprLiteral.hh" -#include "params/TimingExprReadIntReg.hh" #include "params/TimingExprRef.hh" #include "params/TimingExprSrcReg.hh" #include "params/TimingExprUn.hh" @@ -124,19 +123,6 @@ uint64_t eval(TimingExprEvalContext &context); }; -class TimingExprReadIntReg : public TimingExpr -{ - public: - TimingExpr *reg; - - TimingExprReadIntReg(const TimingExprReadIntRegParams &params) : - TimingExpr(params), - reg(params.reg) - { } - - uint64_t eval(TimingExprEvalContext &context); -}; - class TimingExprLet : public TimingExpr { public: -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/49776 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: I253a0a058dc078deeb28ef0babead4c8ffc3b792 Gerrit-Change-Number: 49776 Gerrit-PatchSet: 77 Gerrit-Owner: Gabe Black <gabe.black@gmail.com> Gerrit-Reviewer: Gabe Black <gabe.black@gmail.com> Gerrit-Reviewer: Giacomo Travaglini <giacomo.travaglini@arm.com> Gerrit-Reviewer: Jason Lowe-Power <jason@lowepower.com> Gerrit-Reviewer: kokoro <noreply+kokoro@google.com> Gerrit-MessageType: merged