> On 2011-05-28 22:04:00, Steve Reinhardt wrote: > > This looks fine to me, but I'm confused... don't you delete this code > > completely two patches from now? Why bother changing it if you're going to > > get rid of it? > > Gabe Black wrote: > Because this way both are available, and the ISAs can be converted one at > a time and everything will work between every patch. Otherwise I'd have to > break everything that wasn't (or was) updated, or do everything in one big > change that does too much at once. Once all the ISAs are consistently on the > new system, the old system isn't needed anymore and is deleted in that later > patch.
OK, I see. Overall this seems like overkill; I can see how this code was useful while you were developing, but for committing, one complete patch that gets rid of the old way of doing things and fixes all the ISAs to use the new way would be preferable to me. I think it would be less confusing because all the related changes would be right there in one place. I don't have a problem with large changesets (in terms of lines of code or files touched), just ones that are not conceptually unified, and spreading this change across multiple csets goes against "conceptually unified" in the opposite direction, IMO. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/656/#review1277 ----------------------------------------------------------- On 2011-04-25 03:04:12, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/656/ > ----------------------------------------------------------- > > (Updated 2011-04-25 03:04:12) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > ISA parser: Allow defining operand types with a ctype directly. > > > Diffs > ----- > > src/arch/isa_parser.py de679a068dd8 > > Diff: http://reviews.m5sim.org/r/656/diff > > > Testing > ------- > > > Thanks, > > Gabe > > _______________________________________________ gem5-dev mailing list gem5-dev@m5sim.org http://m5sim.org/mailman/listinfo/gem5-dev
