What is the status of this patch? Nilay, are you trying to address Gabe's concerns or are you having a problem defining a solution that meets both your and Gabe's requirements? I don't understand the specifics of this patch, but I want to do whatever possible to find a solution. This is a critical fix for the O3 x86 CPU model.
Thanks, Brad > -----Original Message----- > From: [email protected] [mailto:gem5-dev- > [email protected]] On Behalf Of Nilay Vaish > Sent: Tuesday, May 08, 2012 2:21 PM > To: Nilay Vaish; Steve Reinhardt; Default; Gabe Black > Subject: Re: [gem5-dev] Review Request: X86: Split Condition Code register > > > > > On May 3, 2012, 9:50 a.m., Nilay Vaish wrote: > > > Should I assume that no one is opposed to this patch in its current > > > form? This would clear my way in further breaking up the condition code > register. > > > > Steve Reinhardt wrote: > > I see that you responded to Gabe's comment, but I didn't see where > Gabe necessarily bought your response. I agree with his concern that, > although this approach doesn't look too bad now, it doesn't really look like > it > will scale to larger numbers of flag sub-registers very cleanly. Some > alternative approach like where the flag sub-registers all live in a common > struct that could be passed around as an entity might be cleaner here. > > > > That said, if we are going to treat these as fully independent registers > (e.g., letting O3 rename them independently) then maybe there is no > alternative to splitting them out this way. > > > > I guess my main point is that it's not clear to me that the discussion > > is > settled here yet (unfortunately). > > > > Gabe Black wrote: > > Yes, I still don't want these to be separate functions. All the > > subregisters > are going to be mutually exclusive bitfields of the same 64 bit register, so > you > can just or them together and mask them apart and pass them in as a single > argument. The function which computes the flag's values wouldn't have to > change at all. As a matter of fact, if you know a register is necessarily > going to > be overwritten entirely (all but the hypothetical ZAPS register) then there's > no reason to even or them in since their current value won't matter. > > Why is masking the registers apart any better than calling separate functions > to set them? > I certainly don't see how this would reduce complexity of code or scale with > the number of registers. > I am assuming that the functions will get inlined. > > > - Nilay > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1166/#review2642 > ----------------------------------------------------------- > > > 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 _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
