----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1166/#review2730 -----------------------------------------------------------
A solid step in the right direction. There are still some issues to address. The most important one is that you don't want to compute a value, put it in an operand the parser recognizes, and then use that operand to compute other values. You're not using the value the operand used to have, but the parser isn't smart enough to know that and assumes it must be a source as well. You'll find a lot of places in the various ISA descriptions where things are put in a temporary variable, assigned to the destination, and then the temporary is used to compute flags. The other less serious style issues should still be addressed though, and be sure to test carefully. src/arch/x86/isa/microops/fpop.isa <http://reviews.gem5.org/r/1166/#comment3078> This alignment is still not necessary. src/arch/x86/isa/microops/regop.isa <http://reviews.gem5.org/r/1166/#comment3079> This will unintentionally make the parser think ccFlagBits is a source. You'll want to use a temporary for psrc1 ^ op2. I think this problem crops up in a lot of this patch. src/arch/x86/isa/microops/regop.isa <http://reviews.gem5.org/r/1166/#comment3080> } on the same line as else - Gabe Black On May 18, 2012, 11:55 a.m., Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1166/ > ----------------------------------------------------------- > > (Updated May 18, 2012, 11:55 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9008:44471dbd9f5c > --------------------------- > X86: Split Condition Code register > This patch moves the ECF and EZF bits to the uccFlagBits and the CF and OF > bits to cfofFlag registers. Ultimately we will have the following registers > [ZAPS], [OF], [CF], [ECF], [EZF] and [DF]. > > > Diffs > ----- > > src/arch/x86/isa/microops/debug.isa 7100059f7bfd > src/arch/x86/isa/microops/fpop.isa 7100059f7bfd > src/arch/x86/isa/microops/mediaop.isa 7100059f7bfd > src/arch/x86/isa/microops/regop.isa 7100059f7bfd > src/arch/x86/isa/microops/seqop.isa 7100059f7bfd > src/arch/x86/isa/microops/specop.isa 7100059f7bfd > src/arch/x86/isa/operands.isa 7100059f7bfd > src/arch/x86/regs/misc.hh 7100059f7bfd > src/arch/x86/x86_traits.hh 7100059f7bfd > > 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
