----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/339/#review518 -----------------------------------------------------------
I won't argue whether this forwarding should be done this way since we already beat that horse to death. I do think all the changes that are being added to the parser here could (and should) be done elsewhere pretty easily. Otherwise this looks like it only needs to be adjusted a little bit and it should be good to go. src/arch/isa_parser.py <http://reviews.m5sim.org/r/339/#comment764> Once the changes below are moved out, this copyright should be removed. src/arch/isa_parser.py <http://reviews.m5sim.org/r/339/#comment762> 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. src/arch/isa_parser.py <http://reviews.m5sim.org/r/339/#comment761> 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. src/cpu/o3/iew_impl.hh <http://reviews.m5sim.org/r/339/#comment763> Don't add these blank lines. src/cpu/o3/lsq_unit_impl.hh <http://reviews.m5sim.org/r/339/#comment765> 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? - Gabe 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
