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
