We're debating how the code in the patch should be organized. My position is still that the function which computes condition codes should not be split up and doesn't actually need to change. It sounds like Steve agrees.
Gabe On 05/10/12 13:31, Beckmann, Brad wrote: > 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 _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
