Without looking at the actual code, the PC operand class sounds fine. For when I get a chance to look at your patches in more depth, what documentation (preferably downloadable) for PowerPC would you recommend?
Gabe Quoting Timothy M Jones <[email protected]>: > Dear all, > > Thanks very much for all the positive comments you've sent about this :-) > > Steve: I'll look into generating different StaticInst objects for those > cases you mentioned. I'll also look redoing the times syscall. > > Gabe: The only changes outside the arch/powerpc directory and syscalls > were to add a new PCOperand class to isa_parser.py because some PowerPC > branch instructions rely on the current value of the PC to compute their > next PC. > > Nate: I'll look at reimplementing the brk syscall like you suggest (I just > didn't think about doing it this way at the time). I'll also go through > all my changes to fix the style as you suggest and see if I can get some > regression tests added in too. > > Ali: Yes, I'm using mercurial queues and I'll try out patchbomb next time. > > Ok, so my plan now is to incorporate your suggestions over the next couple > of weeks and then resend the powerpc-isa, syscall-times and syscall-brk > patches (using patchbomb). > > Thanks for the comments so far. If you have any more, then please do let > me know. > > Cheers > Tim > > On Fri, 28 Aug 2009 15:23:15 +0100, Steve Reinhardt <[email protected]> > wrote: > >> OK, my curiosity got the better of me and I had to take a peek. It >> looks good! I didn't do a comprehensive review, but here are a couple >> little things I noticed: >> >> - You'll want to generate separate StaticInst objects for the rA == 0 >> effective address cases instead of using raZero flag. The problem >> with having a single StaticInst class is that it will list rA as an >> operand, and once you get to O3 then the OOO scheduler will enforce >> that dependence on rA even when it's zero. There's more than one way >> to do this, but if you look at LoadNopCheckDecode in >> arch/alpha/isa/mem.isa you can see how I handled a roughly similar >> situation of making loads that target R31 automatically generate >> no-ops instead of the specified load... that won't work directly for >> you but maybe it will give you some ideas. >> >> - For the times patch, you should find the global >> microseconds-per-tick value; see getrusageFunc() and getElapsedTime() >> in syscall_emul.hh. >> >> BTW, the recommended way to send out patches like this is to use the >> mercurial patchbomb extension: >> http://mercurial.selenic.com/wiki/PatchbombExtension. >> >> Steve >> >> On Fri, Aug 28, 2009 at 6:41 AM, Steve Reinhardt<[email protected]> wrote: >>> Wow, this is a nice surprise! Thanks! >>> >>> I'll try and take a look at the code and give you feedback, but it >>> might not be right away. >>> >>> BTW, you might find this interesting: >>> http://www.m5sim.org/flyspray/task/18. It's actually been on our "to >>> do" list even longer... I think the date is just when we set up the >>> bug database. >>> >>> Steve >>> >>> On Fri, Aug 28, 2009 at 1:26 AM, Timothy M Jones<[email protected]> >>> wrote: >>>> Hi everyone, >>>> >>>> Over the last few weeks I've been working on an implementation of the >>>> PowerPC ISA for M5. Attached is a patch for it. I realise as I'm >>>> writing >>>> this that I should have probably said in advance that I was doing >>>> this, but >>>> there we go. It's done now. >>>> >>>> What it can do: >>>> >>>> * Execute PowerPC binaries on AtomicSimpleCPU. >>>> * Run SpecINT 2000 binaries (when combined with additional system call >>>> patches which I'll attach separately). >>>> >>>> What it can't do: >>>> >>>> * Floating point instructions. I'm hoping to work on that over the next >>>> couple of weeks. >>>> * O3CPU execution. Well, I haven't tried to be honest, but again, I'm >>>> hoping >>>> to get this working too. >>>> * Full system mode. >>>> >>>> I've based it all on the ARM ISA simply because I've worked with this >>>> ISA >>>> before and know a bit about it. However, there may be some code in >>>> there >>>> that is not relevant to PowerPC, but specific to ARM. I haven't done a >>>> thorough check, just enough to get things working. >>>> >>>> To use it you can build a POWERPC_SE executable. For this I also >>>> include my >>>> file from in build_opts. >>>> >>>> If you think that this is useful then I'm more than happy for it to be >>>> incorporated into the main M5 repository. That said, I'm aware that it >>>> might >>>> not be able to be incorporated in its current form. If there are any >>>> changes, additions or anything you'd like me to put in, then please >>>> just let >>>> me know. >>>> >>>> Also, if there are questions about it then I'm more than happy to >>>> answer >>>> them. I'm sure this email doesn't go into enough detail, but I wasn't >>>> sure >>>> how much or what to say about this so I figured that people could ask >>>> what >>>> they don't know. >>>> >>>> I'll now send the system call patches one by one since they are >>>> independent >>>> of the ISA. >>>> >>>> Cheers >>>> Tim >>>> >>>> -- >>>> The University of Edinburgh is a charitable body, registered in >>>> Scotland, with registration number SC005336. >>>> >>>> >>>> TARGET_ISA = 'powerpc' >>>> FULL_SYSTEM = 0 >>>> CPU_MODELS = 'AtomicSimpleCPU' >>>> >>>> _______________________________________________ >>>> m5-dev mailing list >>>> [email protected] >>>> http://m5sim.org/mailman/listinfo/m5-dev >>>> >>>> >>> >> _______________________________________________ >> m5-dev mailing list >> [email protected] >> http://m5sim.org/mailman/listinfo/m5-dev >> > > > > -- > The University of Edinburgh is a charitable body, registered in > Scotland, with registration number SC005336. > > _______________________________________________ > m5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
