My point was that you couldn't define the variables -anywhere- because you can't define variables inside the definition of a dict as far as I know, excluding them from the definition of the operands list, and you couldn't bring them in from anywhere else. It's a moot point now since that's no longer a restriction and I'm going to make the change.
Gabe nathan binkert wrote: > Even if the named constant is in the same file just above where it is > used, that is far better than using the same number in 50 different > places. It is worth it because of exactly what's going on right now. > You're submitting your 2nd 50 lines of diff to change one number in > 50 places. Not cool. > > On Tue, Nov 16, 2010 at 1:34 PM, Gabe Black <[email protected] > <mailto:[email protected]>> wrote: > > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/312/ > > > On November 15th, 2010, 9:46 p.m., *Nathan Binkert* wrote: > > src/arch/arm/isa/operands.isa > <http://reviews.m5sim.org/r/312/diff/1/?file=5179#file5179line89> > (Diff revision 1) > > 89 > > 'FpDest': ('FloatReg', 'sf', '(dest + 0)', 'IsFloating', 3), > > 89 > > 'FpDest': ('FloatReg', 'sf', '(dest + 0)', 'IsFloating', 5), > > 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 > > > On November 15th, 2010, 4:28 a.m., Gabe Black wrote: > > Review request for Default. > By Gabe Black. > > /Updated 2010-11-15 04:28:11/ > > > Description > > 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) > > View Diff <http://reviews.m5sim.org/r/312/diff/> > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
