----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1153/#review3341 -----------------------------------------------------------
Sorry for the delay on this one, Nilay... it looks good. I'm still not thrilled with the amount of duplication in the makeRead() and makeWrite() methods though. I'm fine with having all the instructions use _numSrcRegs++ / _numDestRegs++ as the indices, similar to the way This would allow you to get rid of the predRead and predWrite flags entirely in addition to simplifying the code in makeRead() and makeWrite(). There could be a small performance penalty in debug mode (then again, there might not be or it might be negligible), but I'd be very disappointed if the compiler didn't optimize the difference away entirely for opt or fast. To me that's a reasonable tradeoff. If you really don't want to go there, then one thing you could do that would clean up the code a bit would be to set src_reg_idx to '_sourceIndex++' and dst_reg_idx to '_destIndex++' for operands that are in predicated instructions. Then in makeRead() and makeWrite() you could unconditionally do things like this: int_reg_val = 'xc->readIntRegOperand(this, %s)' % self.src_reg_idx Note that in python '%s' and '%d' do the same thing if the corresponding variable is an integer. This would at least get rid of a lot of the redundancy in the generated code strings, which is especially bad in makeWrite(). It comes at the cost of complicating OperandList.__init__() a bit, since you'd have to make an initial pass to see if predRead or predWrite would be set before being able to set src_reg_idx/dst_reg_idx properly (or you could use a second pass to override src_reg_idx/dst_reg_idx if predRead/predWrite are set). Does that make sense? I'd prefer the former idea (just making everything uniform), but I threw the second one out there as a compromise to be able to generate the same output you're generating now only (IMO) just a little more cleanly. - Steve Reinhardt On July 26, 2012, 8 a.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1153/ > ----------------------------------------------------------- > > (Updated July 26, 2012, 8 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9130:30a74215ca4d > --------------------------- > 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 b57966a6c512 > > 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
