On Mon, Jun 18, 2012 at 4:54 AM, Nilay Vaish <[email protected]> wrote:
> This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1153/ > > On June 17th, 2012, 9:31 p.m., *Steve Reinhardt* wrote: > > Overall, I think that trying to keep both the old model and the new model is > overly complicating things. I think it's fine if all the constructors have > the form: > > _numSrcRegs = 0; > _srcRegIdx[_numSrcRegs++] = <X>; > _srcRegIdx[_numSrcRegs++] = <Y>; > > with optional "if" clauses around the conditional registers. If there are no > if clauses, then compiling with optimization should generate the same code as > the old version where all the indices and values are hard coded (the compiler > can do all the increments at compile time... it's just constant propagation). > The python is complex enough as it is. > > My other concern is that all this only works if the predicate expressions > evaluate the same at construction time as at execution time. I'm not sure > how to enforce that, or if it's practical to do so. I can believe that > that's not a problem for our current usage of this feature, but I can see > that being a very subtle source of bugs in the future. > > > Steve, I have changed the code to address your first concern. > > I understand that the code is getting complicated because of this change. But > then > we do not want any adverse impact on performance of ISAs not using this > feature. > > About your second comment, I agree that I am assuming that expression would > evaluate > the same both the times. I do not know if it is possible to add such a check > in the > isa parser. As a programming practice, we might want to use only const > members of the > instruction in the predicate expression. > > Hi Nilay, Part of my point is that, when compiled with optimization, there should not be any performance difference between the two versions. Thus I don't see a need to maintain two versions, even in the makeRead/makeWrite parts of the code (which I see you haven't updated). Others can weigh in if they disagree. As far as the second issue, I agree there may not be anything to do there besides perhaps add some comments documenting the restriction. The const idea is a good one though it may be unenforceable. Our options might be clearer when we see how this feature is used. All this said, I think in the long run we will still need to handle the x86 condition code specially and not treat them like GPRs, so I'm not yet convinced that this patch is more than an academic exercise. Nevertheless, I can see how it could be useful in the future. Steve _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
