> On 2010-12-08 22:45:44, Gabe Black wrote: > > src/arch/isa_parser.py, line 795 > > <http://reviews.m5sim.org/r/339/diff/1/?file=5449#file5449line795> > > > > These values aren't passed directly to anything, they're exposed > > through a header file and then imported into the ArmISA namespace in > > registers.hh. You could define the max source index to be the value > > generated here plus the dest value. That gets this change out of the parser > > and avoids bloating the instruction objects for other ISAs that don't need > > it.
So you mean don't include max_inst_regs.hh and just put the constants in registers.hh? Ali > On 2010-12-08 22:45:44, Gabe Black wrote: > > src/arch/isa_parser.py, line 1 > > <http://reviews.m5sim.org/r/339/diff/1/?file=5449#file5449line1> > > > > Once the changes below are moved out, this copyright should be removed. Done > On 2010-12-08 22:45:44, Gabe Black wrote: > > src/arch/isa_parser.py, line 940 > > <http://reviews.m5sim.org/r/339/diff/1/?file=5449#file5449line940> > > > > I really really don't like ARM specific code in the isa_parser. This > > should at least be behind a generalized mechanism if not moved out of the > > parser all together. Why couldn't this code be added to the constructor > > template after the %(constructor)s line? There doesn't seem to be anything > > in it that actually requires knowledge from the parser. Done. > On 2010-12-08 22:45:44, Gabe Black wrote: > > src/cpu/o3/iew_impl.hh, line 1264 > > <http://reviews.m5sim.org/r/339/diff/1/?file=5451#file5451line1264> > > > > Don't add these blank lines. Done. > On 2010-12-08 22:45:44, Gabe Black wrote: > > src/cpu/o3/lsq_unit_impl.hh, line 461 > > <http://reviews.m5sim.org/r/339/diff/1/?file=5452#file5452line461> > > > > I'm not quite sure this is right. Ignoring the predicate case, the load > > -has- executed, but its access (or the load itself) faulted. It would be > > appropriate to have this here but conditioned on the predicate actually > > being false. That would be like the store case below. And actually, if the > > instruction faulted, won't all the registers be rolled back anyway? I don't think it hurts anything to have it here, since as you point out, the state is going to be rolled back. It simplifies the flow a bit. I can put it around an if(). - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/339/#review518 ----------------------------------------------------------- On 2010-12-06 16:11:17, Ali Saidi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/339/ > ----------------------------------------------------------- > > (Updated 2010-12-06 16:11:17) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > ARM: Add support for moving predicated false dest operands from sources. > > > Diffs > ----- > > src/arch/arm/isa/insts/misc.isa 2b5fbdcbfb5d > src/arch/isa_parser.py 2b5fbdcbfb5d > src/cpu/o3/dyn_inst.hh 2b5fbdcbfb5d > src/cpu/o3/iew_impl.hh 2b5fbdcbfb5d > src/cpu/o3/lsq_unit_impl.hh 2b5fbdcbfb5d > > Diff: http://reviews.m5sim.org/r/339/diff > > > Testing > ------- > > > Thanks, > > Ali > >
_______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
