> 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, ) > > > Korey Sewell wrote: > 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 ??? > > Deyuan Guo wrote: > Thanks for you reply. > In my opinion, according to 'NPC=PC+8;NNPC=PC+12', the not_taken_code > should be 'NPC=NNPC;NNPC+=4;'. > But then I find that there is no difference when I comment the 2 lines or > not. > The not_taken_code will be added to > build/MIPS_SE/arch/mips/inorder_cpu_exec.cc, under branch likely > instructions, but it seems that it won't affect the executing procedure? > > ---- > We are using dhrystone benchmarks for fixed-point testing, compiled by a > MIPS64 compiler, which generates many instructions such as beql/ld/sd/lld etc. > During the work, we fixed several bugs of inorder cpu model, and commit > the diff here. > But we only test MIPS64 in SE mode, and there are still some problems > about the float-point part, and lld/LLSC instructions. > Btw, is there any plan to support MIPS64 in gem5? > > Korey Sewell wrote: > For the Branch Likely Code, are you sure that your compiler is generating > those type of instructions? Commenting the code out doesn't seem like an > actual solution but more of a avoidance of a bug. If it is a bug, then we > should just fix it. > ---- > From the internal gem5 development team, I dont know of any active MIPS64 > development. I had done some work in the past and followed it up a couple > years ago, but that's not what I'm working. > > I have heard a few questions about MIPS at the tutorial and through the > list recently. Since gem5 is such a community-like project, there are > typically a number of people 'silently' working on a particular extension > (e.g. MIPS64) or wanting that same feature and willing to put in work to > develop (as long as the burden of development isn't overly taxing) > > I'd suggest if you are interested in MIPS64, send out an inquiry to the > gem5-dev list asking who else is interested and also list the features that > aren't implemented currently but that you need for it to work. At the very > minimum, the gem5-dev list can give you advice on where to start your > development but in the ideal case someone else will want the same feature and > you can team up on development.
In the future, you can try updating your current changeset (you can do this w/the reviewboard command line: hg postreview -e -u <#> -o). That will save you having to post a new patch and delete the old one. - 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
