> On 2010-08-23 11:13:37, Gabe Black wrote: > > src/arch/arm/utility.cc, line 62 > > <http://reviews.m5sim.org/r/228/diff/1/?file=2112#file2112line62> > > > > I realize the argument registers are just counting up from 0, but since > > you do define constants for them it might be better to put them in an array > > and use argument[number] as the register index. Otherwise if someone were > > to change ArgumentReg0 to 5, nothing (or something weird) would happen. > > Also you might want to put NumArgumentRegs after the ArgumentRegs, although > > it's not a big deal. > > > > I'd also suggest some wording changes for the fatals. > > > > "getArgument() doesn't support fp regs!\n" => > > "getArgument(): Floating point arguments not implemented.\n" > > > > "getArgument() not implemented for larger than NumArgumentRegs!\n" => > > "getArgument(): Argument index %d beyond max supported (%d).\n", > > number, NumArgumentRegs - 1, > > > > "getArgument() only implemented for FULL_SYSTEM\n" => > > "getArgument(): Not supported in SE mode.\n" > > > > I don't know how the code in the other files normally works, but I > > didn't see anything terribly wrong with it.
Well, if someone does that it's horribly wrong. You can do if (ArgumentReg0 + number < ArgumentReg0 + NumArgumentRegs), but that doesn't seem like a worthwhile change. if someone decides to change the ABI more than this is going to break. I changed the panics. - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/228/#review311 ----------------------------------------------------------- On 2010-08-23 09:35:37, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/228/ > ----------------------------------------------------------- > > (Updated 2010-08-23 09:35:37) > > > Review request for Default. > > > Summary > ------- > > ARM: Limited implementation of dprintk. > > Does not work with vfp arguments or arguments passed on the stack. > > > Diffs > ----- > > src/arch/arm/registers.hh 47d9409b2b7f > src/arch/arm/system.hh 47d9409b2b7f > src/arch/arm/system.cc 47d9409b2b7f > src/arch/arm/utility.cc 47d9409b2b7f > > Diff: http://reviews.m5sim.org/r/228/diff > > > Testing > ------- > > > Thanks, > > Ali > > _______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
