> On 2011-06-13 07:27:06, Korey Sewell wrote: > > src/cpu/inorder/resources/bpred_unit.cc, line 377 > > <http://reviews.m5sim.org/r/744/diff/1/?file=13011#file13011line377> > > > > Can you post a separate patch for the MipsISA stuff you are describing? > > I dont see anything in the DIFF. Also, can you list benchmarks that you > > have observed to work? > > > > My overall comment is that I think I caught #s1-3 in the new InOrderCPU > > code (the code to get Full System working). > > > > It's just stuck behind the patches on the reviewboard being merged and > > passing the regressions. It's predominantly done now, w/the DTB patch being > > closed to resolved. The last hiccup seems to be that the 50.vortex > > regressions seems to have been broken in the merge. Once that gets > > resolved, we'll get a wave of inorder updates. > > > > At that point (in the near future), we'll compare the branch code and > > I'm hoping that you (Deyuan Guo) would be OK retesting your branch changes. > > Deyuan Guo wrote: > diff -r 9fb150de362e src/arch/mips/isa/formats/branch.isa > --- a/src/arch/mips/isa/formats/branch.isa Sun Jun 12 23:52:21 > 2011 -0700 > +++ b/src/arch/mips/isa/formats/branch.isa Tue Jun 14 14:13:47 > 2011 +0800 > @@ -234,7 +234,7 @@ > if x == 'Link': > code += 'R31 = NNPC;\n' > elif x == 'Likely': > - not_taken_code = 'NNPC = NPC; NPC = PC;' > + # not_taken_code = 'NNPC = NPC; NPC = PC;' > inst_flags += ('IsCondDelaySlot', ) > else: > inst_flags += (x, ) > @@ -273,7 +273,7 @@ > if x == 'Link': > code += 'R32 = NNPC;' > elif x == 'Likely': > - not_taken_code = 'NNPC = NPC, NPC = PC;' > + # not_taken_code = 'NNPC = NPC, NPC = PC;' > inst_flags += ('IsCondDelaySlot', ) > else: > inst_flags += (x, ) >
For future reference, this is best placed in a separate patch on the reviewboard, but this seems ok since it's a small diff. I'm curious as to why this doesn't work? In a Conditional delay slot, if you're branch isn't taken, then the next PC should be the NNPC right?... As in taken: NPC = PC+4 (sizeof instruction) NNPC = target And not taken: NPC = PC+8 NNPC = PC+12 ARe you seeing that the not taken code is generating a duplicate instruction or ??? - Korey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/744/#review1328 ----------------------------------------------------------- On 2011-06-13 01:11:53, Deyuan Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/744/ > ----------------------------------------------------------- > > (Updated 2011-06-13 01:11:53) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > We are porting gem5 to support MIPS64, and find some problems. We try to fix > them, as shown below: > > 1. fetch_seq_unit.cc > Comment 2 lines. pc shouldn't be changed at this condition; > 2. branch_predictor.cc > Comment 1 line. By default, set target to NPC is OK. > Adding a condition for MIPS branch likely instruction. > 3. bpred_unit.cc > If the branch is not taken, BTB should not be updated. > > In addition, 2 places in src/arch/mips/isa/formats/branch.isa: > elif x == 'Likely': > # not_taken_code = 'NNPC = NPC, NPC = PC;' > The not_taken_code should be commented. > > > Diffs > ----- > > src/cpu/inorder/resources/bpred_unit.cc 9fb150de362e > src/cpu/inorder/resources/branch_predictor.cc 9fb150de362e > src/cpu/inorder/resources/fetch_seq_unit.cc 9fb150de362e > > Diff: http://reviews.m5sim.org/r/744/diff > > > Testing > ------- > > We have run many benchmarks of MIPS64 in gem5. > And for MIPS32, with CPU_MODELS=InOrderCPU, hello world run correctly. > > > Thanks, > > Deyuan > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
