> On 2010-11-15 21:46:47, Nathan Binkert wrote: > > src/arch/arm/isa/operands.isa, line 89 > > <http://reviews.m5sim.org/r/312/diff/1/?file=5179#file5179line89> > > > > Ok, I thought I commented on this last time. This is nuts. You can't > > have this random number here that needs to change whenever you do something.
The number is for sorting the operands when they're written out/read in/etc. It being a raw number is historical in two respects. First, that's how Alpha did it, and that's how all the other ISAs inherited dong it. Second, the operands construct used to be defined in an isolated scope, or in other words the dict used as its global and/or local scope didn't have much in it, didn't carry over from other parts of the description, etc. That made it essentially impossible to use any variables defined elsewhere. That's no longer true, though, since I opened it up to allow defining the shorthand functions in one of the ISAs, although I forget which one. I think normally the numbers would be sequential, but ARM has them set up to mostly not do anything. I half remember that the only reason there isn't a single value all together is that I needed to make sure the indexes were ordered in a particular way when generating disassembly output, and then later to make sure interacting writes happened in the right order. Anyway, I think using named constants would make sense here, although it goes against the grain of the other ISAs and wouldn't necessarily fit with all of them very well since they don't all reuse values like this. I'll make the change before I check this in. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/312/#review483 ----------------------------------------------------------- On 2010-11-15 04:28:11, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/312/ > ----------------------------------------------------------- > > (Updated 2010-11-15 04:28:11) > > > Review request for Default. > > > Summary > ------- > > ARM: Take advantage of new PCState syntax. > > > Diffs > ----- > > src/arch/arm/isa/insts/branch.isa f440cdaf1c2d > src/arch/arm/isa/insts/data.isa f440cdaf1c2d > src/arch/arm/isa/insts/ldr.isa f440cdaf1c2d > src/arch/arm/isa/insts/macromem.isa f440cdaf1c2d > src/arch/arm/isa/insts/misc.isa f440cdaf1c2d > src/arch/arm/isa/operands.isa f440cdaf1c2d > > Diff: http://reviews.m5sim.org/r/312/diff > > > Testing > ------- > > > Thanks, > > Gabe > >
_______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
