Review Board 2.0.15


cpu: Simplify the rename interface and use RegId

Review Request #3754 - Created Dec. 9, 2016 and updated

Information
Rekai Gonzalez Alberquilla
gem5
default
Reviewers
Default 
Changeset 11760:686a5d1464b6
---------------------------
cpu: Simplify the rename interface and use RegId

With the hierarchical RegId there are a lot of functions that are
redundant now.

The idea behind the simplification is that instead of having the regId,
telling which kind of register read/write/rename/lookup/etc. and then
the function panic_if'ing if the regId is not of the appropriate type,
what we ahve is an interface that you give it the regId, and depending
on the type of register, it decides which kind of register to read...

Change-Id: I7d52e9e21fc01205ae365d86921a4ceb67a57178
Reviewed-by: Andreas Sandberg <andreas.sandberg@arm.com>
Built in regressions passing

Issue Summary

1 0 0 1
Description From Last Updated Status
Review request changed
Updated (Jan. 16, 2017, 3:46 a.m.)

Status: Re-opened

Summary:
-cpu: Generalize rename to use RegId
+cpu: Simplify the rename interface and use RegId

Description:
~  

Changeset 11760:ed27616db928

  ~

Changeset 11760:686a5d1464b6

   
~  

cpu: Generalize rename to use RegId

  ~

cpu: Simplify the rename interface and use RegId

   
~  

Generalization of the rename machinery to work on the newly added

~   RegIds.

  ~

With the hierarchical RegId there are a lot of functions that are

  ~ redundant now.

   
~  

Change-Id: I9bf1f12bb357e42302fb875d3a2e9ce0bf2518c1

  ~

The idea behind the simplification is that instead of having the regId,

  + telling which kind of register read/write/rename/lookup/etc. and then
  + the function panic_if'ing if the regId is not of the appropriate type,
  + what we ahve is an interface that you give it the regId, and depending
  + on the type of register, it decides which kind of register to read...

  +
  +

Change-Id: I7d52e9e21fc01205ae365d86921a4ceb67a57178

    Reviewed-by: Andreas Sandberg andreas.sandberg@arm.com

Testing Done:
  +

Built in regressions passing

Diff:

Revision 1 (+748 -759)

src/arch/alpha/isa.hh
src/arch/alpha/isa/branch.isa
src/arch/alpha/isa/fp.isa
src/arch/alpha/isa/main.isa
src/arch/arm/isa.hh
src/arch/arm/isa.cc
src/arch/arm/insts/misc.cc
src/arch/arm/isa/insts/data64.isa
54 more
Jason Lowe-Power
Posted (Jan. 19, 2017, 2:13 p.m.)
I've looked through the patch... but I really don't know anything about this code. Other than the small question below, it looks OK to me.
src/cpu/reg_class_impl.hh (Diff revision 1) 
 
 
 
Why is this an implemenation header file?
Show all issues 
  1. Because it is something that can/should be inlined, as the control has dependence on things that can be optimized at compile-time. On the other hand, as it depends on 'TheISA', having the implementation in the header file would make the reg_class.hh depend on the ISA, and that leads to some compile time issues due to cyclic dependencies. Having as separate implementation file is a compromise solution that keeps the dependencies to the minimum required, while still providing the code for inlining to have the minimum impact on performance.
Ship it!
Jason Lowe-Power
Posted (Jan. 24, 2017, 7:30 a.m.)
Ship It!