> On June 17, 2012, 9:31 p.m., Steve Reinhardt wrote: > > Overall, I think that trying to keep both the old model and the new model > > is overly complicating things. I think it's fine if all the constructors > > have the form: > > > > _numSrcRegs = 0; > > _srcRegIdx[_numSrcRegs++] = <X>; > > _srcRegIdx[_numSrcRegs++] = <Y>; > > > > with optional "if" clauses around the conditional registers. If there are > > no if clauses, then compiling with optimization should generate the same > > code as the old version where all the indices and values are hard coded > > (the compiler can do all the increments at compile time... it's just > > constant propagation). The python is complex enough as it is. > > > > My other concern is that all this only works if the predicate expressions > > evaluate the same at construction time as at execution time. I'm not sure > > how to enforce that, or if it's practical to do so. I can believe that > > that's not a problem for our current usage of this feature, but I can see > > that being a very subtle source of bugs in the future. > > > >
Steve, I have changed the code to address your first concern. I understand that the code is getting complicated because of this change. But then we do not want any adverse impact on performance of ISAs not using this feature. About your second comment, I agree that I am assuming that expression would evaluate the same both the times. I do not know if it is possible to add such a check in the isa parser. As a programming practice, we might want to use only const members of the instruction in the predicate expression. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1153/#review2951 ----------------------------------------------------------- On June 18, 2012, 3:44 a.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1153/ > ----------------------------------------------------------- > > (Updated June 18, 2012, 3:44 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9071:b1df7f254c3a > --------------------------- > ISA Parser: Allow predication of source and destination registers > This patch is meant for allowing predicated reads and writes. Note that this > predication is different from the ISA provided predication. They way we > currently provide the ISA description for X86, we read/write registers that > do not need to be actually read/written. This is likely to be true for other > ISAs as well. This patch allows for read and write predicates to be associated > with operands. It allows for the register indices for source and destination > registers to be decided at the time when the microop is constructed. The > run time indicies come in to play only when the at least one of the > predicates has been provided. This patch will not affect any of the ISAs that > do not provide these predicates. Also the patch assumes that the order in > which operands appear in any function of the microop is same across all the > functions of the microops. A subsequent patch will enable predication for the > x86 ISA. > > > Diffs > ----- > > src/arch/isa_parser.py fa77985a87c6 > > Diff: http://reviews.gem5.org/r/1153/diff/ > > > Testing > ------- > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
