> On April 28, 2012, 10:11 p.m., Gabe Black wrote: > > src/arch/x86/insts/microregop.cc, line 80 > > <http://reviews.gem5.org/r/1166/diff/1/?file=26160#file26160line80> > > > > I don't think this should be split out into a separate function. > > Nilay Vaish wrote: > the reason being? > > Gabe Black wrote: > Sorry, I should have explained. Lets say we have 6 flag registers like I > think we're going to, and for simplicity I'll call the A, B, C, D, E, and F. > If we have a separate function for each, that will look something like the > following: > > void setA(...) > { > ... > } > > void setB(...) > { > ... > } > > etc. > > setA() > serB() > setC() > setD() > setE() > setF() > > compare that to still having a single function which sets all the > registers together. In one case we have six calls to six small functions, and > in the other we have one call to a larger function. Since the pieces of the > larger function make sense to put together (as opposed to random stuff being > shoved into a single function), the primary effects are that we save the > overhead of 5 function calls, and we can avoid some of the redundancies we'd > get from the smaller functions like ECF and EZF being essentially the same as > the code for CF and ZF. It's possible the compiler would figure out that > those smaller functions could be inlined into a larger function behind the > scenes anyway, but I think the code is cleaner and simpler if we keep things > together.
I had thought of keeping a single function, but it seems a couple of things go against this choice. Firstly, the number of arguments will go up. Secondly, currently we return the newly computed value. If we update all the registers in a single function, then we will have to pass these by reference. I don't know if the ISA parser will still recognise that these registers are being written on to. - Nilay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1166/#review2617 ----------------------------------------------------------- On April 27, 2012, 8:03 a.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1166/ > ----------------------------------------------------------- > > (Updated April 27, 2012, 8:03 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 8973:e70eb4904db5 > --------------------------- > X86: Split Condition Code register > This patch moves the ECF and EZF bits to a separate register. This is just > an example patch for discussion. This is how I am proposing we should split > the register. If this is acceptable, then ultimately we will have the > following registers -- [ZAPS], [OF], [CF], [ECF], [EZF] and [DF]. > > > Diffs > ----- > > src/arch/x86/insts/microregop.hh b9f4e3884951 > src/arch/x86/insts/microregop.cc b9f4e3884951 > src/arch/x86/isa/microops/debug.isa b9f4e3884951 > src/arch/x86/isa/microops/fpop.isa b9f4e3884951 > src/arch/x86/isa/microops/regop.isa b9f4e3884951 > src/arch/x86/isa/microops/seqop.isa b9f4e3884951 > src/arch/x86/isa/microops/specop.isa b9f4e3884951 > src/arch/x86/isa/operands.isa b9f4e3884951 > src/arch/x86/x86_traits.hh b9f4e3884951 > > Diff: http://reviews.gem5.org/r/1166/diff/ > > > Testing > ------- > > Boots Linux with atomic cpu. > > > Thanks, > > Nilay Vaish > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
