> On 2011-09-29 14:18:39, Nathan Binkert wrote: > > While this might looks somewhat like the CRTP, it's a pretty wierd version > > of it, such that I probably wouldn't even call it that. The fact that you > > don't even use the template parameter is pretty odd. > > > > As far as I can tell, you added the template so you could avoid having to > > declare the various static members for each version of the class. I'd > > personally think it's pretty confusing. > > > > Wouldn't it be simpler to get rid of just about all of this code, including > > the accessors and just have a single public member that is a "static const > > FaultVals vals"? For each fault? > > > > If you really don't want to do this, I won't stand in your way (because I > > do believe done > perfect), but it took a long time for me to figure out > > what the heck was going on.
Isn't that how this is supposed to work? I did the same thing here that I was told to do a long time ago for SPARC and what was called CRTP back by you guys back then. I made the change to Alpha and MIPS to be consistent, and because they had basically the same code I was told to clean up in SPARC years ago. Being able to declare a fault class in one line is pretty nice too. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/882/#review1592 ----------------------------------------------------------- On 2011-09-25 03:04:11, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/882/ > ----------------------------------------------------------- > > (Updated 2011-09-25 03:04:11) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > Alpha: Use the CRTP to simplify Alpha's fault classes. > > > Diffs > ----- > > src/arch/alpha/ev5.cc e21224136182 > src/arch/alpha/faults.hh e21224136182 > src/arch/alpha/faults.cc e21224136182 > > Diff: http://reviews.m5sim.org/r/882/diff > > > Testing > ------- > > > Thanks, > > Gabe > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
