Andreas, can you point out what issue has not been addressed in my responses? In my opinion everything has been.

--
Nilay

On Mon, 27 Jul 2015, Andreas Hansson wrote:

Hi Nilay,

Can we please revert this changeset and leave the review open? As already
stated in the review comments, there are some critical issues that lead us
to believe that this patch does not bring us closer to a solution.

It would be much appreciated if we can keep this code in review until all
these issues are resolved and we have established a solution that works
more broadly.

Thanks,

Andreas

On 26/07/2015 16:21, "gem5-dev on behalf of Nilay Vaish"
<[email protected] on behalf of [email protected]> wrote:

changeset 5af8f40d8f2c in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=5af8f40d8f2c
description:
      cpu: implements vector registers

      This adds a vector register type.  The type is defined as a std::array
of a
      fixed number of uint64_ts.  The isa_parser.py has been modified to parse
vector
      register operands and generate the required code.  Different cpus have
vector
      register files now.

diffstat:

src/arch/SConscript                 |    4 +-
src/arch/alpha/isa.hh               |    7 ++
src/arch/alpha/registers.hh         |   10 ++-
src/arch/alpha/utility.cc           |    1 +
src/arch/arm/insts/static_inst.cc   |    2 +
src/arch/arm/isa.hh                 |    7 ++
src/arch/arm/registers.hh           |   10 ++-
src/arch/arm/utility.cc             |    3 +
src/arch/isa_parser.py              |  115
+++++++++++++++++++++++++++++++++++-
src/arch/mips/isa.hh                |    7 ++
src/arch/mips/registers.hh          |   10 ++-
src/arch/mips/utility.cc            |    3 +
src/arch/null/registers.hh          |    2 +
src/arch/power/insts/static_inst.cc |    2 +
src/arch/power/isa.hh               |    7 ++
src/arch/power/registers.hh         |   10 ++-
src/arch/power/utility.cc           |    3 +
src/arch/sparc/isa.hh               |    7 ++
src/arch/sparc/registers.hh         |    9 ++-
src/arch/sparc/utility.cc           |    3 +
src/arch/x86/insts/static_inst.cc   |    7 ++
src/arch/x86/isa.hh                 |    6 +
src/arch/x86/registers.hh           |   11 +++-
src/arch/x86/utility.cc             |    4 +
src/cpu/StaticInstFlags.py          |    7 +-
src/cpu/base_dyn_inst.hh            |   26 ++++++++
src/cpu/checker/cpu.hh              |   24 +++++++-
src/cpu/checker/cpu_impl.hh         |   74 ++++++++++++++++------
src/cpu/checker/thread_context.hh   |   16 +++++
src/cpu/exec_context.hh             |   17 +++++
src/cpu/minor/dyn_inst.cc           |    2 +
src/cpu/minor/exec_context.hh       |   43 +++++++++----
src/cpu/minor/scoreboard.cc         |    8 ++
src/cpu/minor/scoreboard.hh         |   12 ++-
src/cpu/o3/O3CPU.py                 |    7 ++
src/cpu/o3/cpu.cc                   |   76 +++++++++++++++++++++++-
src/cpu/o3/cpu.hh                   |   12 +++
src/cpu/o3/dyn_inst.hh              |   19 +++++-
src/cpu/o3/free_list.hh             |   21 +++++-
src/cpu/o3/inst_queue_impl.hh       |    2 +-
src/cpu/o3/regfile.cc               |   26 ++++++-
src/cpu/o3/regfile.hh               |   52 +++++++++++++++-
src/cpu/o3/rename_impl.hh           |   20 +++++-
src/cpu/o3/rename_map.cc            |   12 +++
src/cpu/o3/rename_map.hh            |   41 ++++++++++++-
src/cpu/o3/thread_context.hh        |   12 +++
src/cpu/o3/thread_context_impl.hh   |   23 +++++++
src/cpu/reg_class.cc                |    1 +
src/cpu/reg_class.hh                |    6 +-
src/cpu/simple/base.hh              |   20 ++++++
src/cpu/simple_thread.hh            |   50 +++++++++++++++
src/cpu/static_inst.hh              |   12 ++-
src/cpu/thread_context.cc           |   30 +++++++++
src/cpu/thread_context.hh           |   24 +++++++
src/sim/insttracer.hh               |    9 ++-
55 files changed, 876 insertions(+), 78 deletions(-)

diffs (truncated from 2354 to 300 lines):

diff -r e1309937d313 -r 5af8f40d8f2c src/arch/SConscript
--- a/src/arch/SConscript      Sun Jul 26 10:20:07 2015 -0500
+++ b/src/arch/SConscript      Sun Jul 26 10:21:20 2015 -0500
@@ -196,5 +196,7 @@
DebugFlag('IntRegs')
DebugFlag('FloatRegs')
DebugFlag('CCRegs')
+DebugFlag('VectorRegs')
DebugFlag('MiscRegs')
-CompoundFlag('Registers', [ 'IntRegs', 'FloatRegs', 'CCRegs', 'MiscRegs'
])
+CompoundFlag('Registers', [ 'IntRegs', 'FloatRegs', 'CCRegs',
'VectorRegs',
+                            'MiscRegs' ])
diff -r e1309937d313 -r 5af8f40d8f2c src/arch/alpha/isa.hh
--- a/src/arch/alpha/isa.hh    Sun Jul 26 10:20:07 2015 -0500
+++ b/src/arch/alpha/isa.hh    Sun Jul 26 10:21:20 2015 -0500
@@ -114,6 +114,13 @@
            return reg;
        }

+        // dummy
+        int
+        flattenVectorIndex(int reg) const
+        {
+            return reg;
+        }
+
        int
        flattenMiscIndex(int reg) const
        {
diff -r e1309937d313 -r 5af8f40d8f2c src/arch/alpha/registers.hh
--- a/src/arch/alpha/registers.hh      Sun Jul 26 10:20:07 2015 -0500
+++ b/src/arch/alpha/registers.hh      Sun Jul 26 10:21:20 2015 -0500
@@ -56,6 +56,12 @@
// dummy typedef since we don't have CC regs
typedef uint8_t CCReg;

+// vector register file entry type
+typedef uint64_t VectorRegElement;
+const int NumVectorRegElements = 0;
+const int VectorRegBytes = NumVectorRegElements *
sizeof(VectorRegElement);
+typedef std::array<VectorRegElement, NumVectorRegElements> VectorReg;
+
union AnyReg
{
    IntReg  intreg;
@@ -95,6 +101,7 @@
const int NumIntRegs = NumIntArchRegs + NumPALShadowRegs;
const int NumFloatRegs = NumFloatArchRegs;
const int NumCCRegs = 0;
+const int NumVectorRegs = 0;
const int NumMiscRegs = NUM_MISCREGS;

const int TotalNumRegs =
@@ -106,7 +113,8 @@
    // 32..63 are the FP regs 0..31, i.e. use (reg + FP_Reg_Base)
    FP_Reg_Base = NumIntRegs,
    CC_Reg_Base = FP_Reg_Base + NumFloatRegs,
-    Misc_Reg_Base = CC_Reg_Base + NumCCRegs, // NumCCRegs == 0
+    Vector_Reg_Base = CC_Reg_Base + NumCCRegs, // NumCCRegs == 0
+    Misc_Reg_Base = Vector_Reg_Base + NumCCRegs, // NumVectorRegs == 0
    Max_Reg_Index = Misc_Reg_Base + NumMiscRegs + NumInternalProcRegs
};

diff -r e1309937d313 -r 5af8f40d8f2c src/arch/alpha/utility.cc
--- a/src/arch/alpha/utility.cc        Sun Jul 26 10:20:07 2015 -0500
+++ b/src/arch/alpha/utility.cc        Sun Jul 26 10:21:20 2015 -0500
@@ -73,6 +73,7 @@

    // Would need to add condition-code regs if implemented
    assert(NumCCRegs == 0);
+    assert(NumVectorRegs == 0);

    // Copy misc. registers
    copyMiscRegs(src, dest);
diff -r e1309937d313 -r 5af8f40d8f2c src/arch/arm/insts/static_inst.cc
--- a/src/arch/arm/insts/static_inst.cc        Sun Jul 26 10:20:07 2015 -0500
+++ b/src/arch/arm/insts/static_inst.cc        Sun Jul 26 10:21:20 2015 -0500
@@ -337,6 +337,8 @@
      case CCRegClass:
        ccprintf(os, "cc_%s", ArmISA::ccRegName[rel_reg]);
        break;
+      case VectorRegClass:
+        panic("ARM ISA does not have any vector registers yet!");
    }
}

diff -r e1309937d313 -r 5af8f40d8f2c src/arch/arm/isa.hh
--- a/src/arch/arm/isa.hh      Sun Jul 26 10:20:07 2015 -0500
+++ b/src/arch/arm/isa.hh      Sun Jul 26 10:21:20 2015 -0500
@@ -288,6 +288,13 @@
        }

        int
+        flattenVectorIndex(int reg) const
+        {
+            assert(reg >= 0);
+            return reg;
+        }
+
+        int
        flattenMiscIndex(int reg) const
        {
            assert(reg >= 0);
diff -r e1309937d313 -r 5af8f40d8f2c src/arch/arm/registers.hh
--- a/src/arch/arm/registers.hh        Sun Jul 26 10:20:07 2015 -0500
+++ b/src/arch/arm/registers.hh        Sun Jul 26 10:21:20 2015 -0500
@@ -72,6 +72,12 @@
// condition code register; must be at least 32 bits for FpCondCodes
typedef uint64_t CCReg;

+// vector register file entry type
+typedef uint64_t VectorRegElement;
+const int NumVectorRegElements = 0;
+const int VectorRegBytes = NumVectorRegElements *
sizeof(VectorRegElement);
+typedef std::array<VectorRegElement, NumVectorRegElements> VectorReg;
+
// Constants Related to the number of registers
const int NumIntArchRegs = NUM_ARCH_INTREGS;
// The number of single precision floating point registers
@@ -82,6 +88,7 @@
const int NumIntRegs = NUM_INTREGS;
const int NumFloatRegs = NumFloatV8ArchRegs + NumFloatSpecialRegs;
const int NumCCRegs = NUM_CCREGS;
+const int NumVectorRegs = 0;
const int NumMiscRegs = NUM_MISCREGS;

#define ISA_HAS_CC_REGS
@@ -112,7 +119,8 @@
// These help enumerate all the registers for dependence tracking.
const int FP_Reg_Base = NumIntRegs * (MODE_MAXMODE + 1);
const int CC_Reg_Base = FP_Reg_Base + NumFloatRegs;
-const int Misc_Reg_Base = CC_Reg_Base + NumCCRegs;
+const int Vector_Reg_Base = CC_Reg_Base + NumCCRegs;
+const int Misc_Reg_Base = Vector_Reg_Base + NumVectorRegs;
const int Max_Reg_Index = Misc_Reg_Base + NumMiscRegs;

typedef union {
diff -r e1309937d313 -r 5af8f40d8f2c src/arch/arm/utility.cc
--- a/src/arch/arm/utility.cc  Sun Jul 26 10:20:07 2015 -0500
+++ b/src/arch/arm/utility.cc  Sun Jul 26 10:21:20 2015 -0500
@@ -156,6 +156,9 @@
    for (int i = 0; i < NumCCRegs; i++)
        dest->setCCReg(i, src->readCCReg(i));

+    // Copy vector registers when vector registers put to use.
+    assert(NumVectorRegs == 0);
+
    for (int i = 0; i < NumMiscRegs; i++)
        dest->setMiscRegNoEffect(i, src->readMiscRegNoEffect(i));

diff -r e1309937d313 -r 5af8f40d8f2c src/arch/isa_parser.py
--- a/src/arch/isa_parser.py   Sun Jul 26 10:20:07 2015 -0500
+++ b/src/arch/isa_parser.py   Sun Jul 26 10:21:20 2015 -0500
@@ -515,6 +515,9 @@
    def isCCReg(self):
        return 0

+    def isVectorReg(self):
+        return 0
+
    def isControlReg(self):
        return 0

@@ -751,6 +754,106 @@

        return wb

+class VectorRegOperand(Operand):
+    def isReg(self):
+        return 1
+
+    def isVectorReg(self):
+        return 1
+
+    def __init__(self, parser, full_name, ext, is_src, is_dest):
+        ## Vector registers are always treated as source registers since
+        ## not the whole of them might be written, in which case we need
+        ## to retain the earlier value.
+        super(VectorRegOperand, self).__init__(parser, full_name, ext,
+                                               True, is_dest)
+        self.size = 0
+
+    def finalize(self, predRead, predWrite):
+        self.flags = self.getFlags()
+        self.constructor = self.makeConstructor(predRead, predWrite)
+        self.op_decl = self.makeDecl()
+
+        if self.is_src:
+            self.op_rd = self.makeRead(predRead)
+            self.op_src_decl = self.makeDecl()
+        else:
+            self.op_rd = ''
+            self.op_src_decl = ''
+
+        if self.is_dest:
+            self.op_wb = self.makeWrite(predWrite)
+            self.op_dest_decl = self.makeDecl()
+        else:
+            self.op_wb = ''
+            self.op_dest_decl = ''
+
+    def makeConstructor(self, predRead, predWrite):
+        c_src = ''
+        c_dest = ''
+
+        if self.is_src:
+            c_src = '\n\t_srcRegIdx[_numSrcRegs++] = %s +
Vector_Reg_Base;' % \
+                    (self.reg_spec)
+            if self.hasReadPred():
+                c_src = '\n\tif (%s) {%s\n\t}' % \
+                        (self.read_predicate, c_src)
+
+        if self.is_dest:
+            c_dest = '\n\t_destRegIdx[_numDestRegs++] = %s +
Vector_Reg_Base;' % \
+                    (self.reg_spec)
+            c_dest += '\n\t_numVectorDestRegs++;'
+            if self.hasWritePred():
+                c_dest = '\n\tif (%s) {%s\n\t}' % \
+                         (self.write_predicate, c_dest)
+
+        return c_src + c_dest
+
+    def makeRead(self, predRead):
+        if self.read_code != None:
+            return self.buildReadCode('readVectorRegOperand')
+
+        vector_reg_val = ''
+        if predRead:
+            vector_reg_val = 'xc->readVectorRegOperand(this,
_sourceIndex++)'
+            if self.hasReadPred():
+                vector_reg_val = '(%s) ? %s : 0' % \
+                              (self.read_predicate, vector_reg_val)
+        else:
+            vector_reg_val = 'xc->readVectorRegOperand(this, %d)' % \
+                             self.src_reg_idx
+
+        return '%s = %s;\n' % (self.base_name, vector_reg_val)
+
+    def makeWrite(self, predWrite):
+        if self.write_code != None:
+            return self.buildWriteCode('setVectorRegOperand')
+
+        if predWrite:
+            wp = 'true'
+            if self.hasWritePred():
+                wp = self.write_predicate
+
+            wcond = 'if (%s)' % (wp)
+            windex = '_destIndex++'
+        else:
+            wcond = ''
+            windex = '%d' % self.dest_reg_idx
+
+        wb = '''
+        %s
+        {
+            TheISA::VectorReg final_val = %s;
+            xc->setVectorRegOperand(this, %s, final_val);\n
+            if (traceData) { traceData->setData(final_val); }
+        }''' % (wcond, self.base_name, windex)
+
+        return wb
+
+    def makeDecl(self):
+        ctype = 'TheISA::VectorReg'
+        return '%s %s;\n' % (ctype, self.base_name)
+
class ControlRegOperand(Operand):
    def isReg(self):
        return 1
@@ -818,7 +921,10 @@
        # Note that initializations in the declarations are solely
        # to avoid 'uninitialized variable' errors from the compiler.
        # Declare memory data variable.
-        return '%s %s = 0;\n' % (self.ctype, self.base_name)
+        if 'IsVector' in self.flags:
+            return 'TheISA::VectorReg %s;\n' % self.base_name
+        else:
+            return '%s %s = 0;\n' % (self.ctype, self.base_name)

    def makeRead(self, predRead):
        if self.read_code != None:
@@ -909,6 +1015,7 @@
        self.numFPDestRegs = 0
        self.numIntDestRegs = 0
        self.numCCDestRegs = 0
+        self.numVectorDestRegs = 0
        self.numMiscDestRegs = 0
        self.memOperand = None

@@ -931,6 +1038,8 @@
                        self.numIntDestRegs += 1
                    elif op_desc.isCCReg():
                        self.numCCDestRegs += 1
+                    elif op_desc.isVectorReg():
+                        self.numVectorDestRegs += 1
                    elif op_desc.isControlReg():
                        self.numMiscDestRegs += 1
            elif op_desc.isMem():
@@ -1127,6 +1236,7 @@
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev


-- 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.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in 
England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, 
Registered in England & Wales, Company No:  2548782
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to