Re: [m5-dev] Review Request: inorder: replace schedEvent() code with reschedule().
> On 2011-01-04 16:56:58, Ali Saidi wrote: > > My only question is does it work? it seems like the if statements don't > > translate exactly, but that could just be over-specifying the if (e.g. can > > you ever fall out with out any action?). You're right, the issue is that the old code would silently do nothing if the event was already scheduled and not squashed, where the new code will forcibly reschedule a scheduled event whether it's squashed or not. I discussed this with Korey and he thought it was an oversight, but I since added 'assert(!scheduled() || squashed());' to each of these functions and re-ran the quick regressions with no ill effects. So by that limited definition, yes, it works... - Steve --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/369/#review617 --- On 2011-01-04 14:41:12, Steve Reinhardt wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/369/ > --- > > (Updated 2011-01-04 14:41:12) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > --- > > inorder: replace schedEvent() code with reschedule(). > There were several copies of similar functions that looked > like they all replicated reschedule(), so I replaced them > with direct calls. Keeping this separate from the previous > cset since there may be some subtle functional differences > if the code ever reschedules an event that is scheduled but > not squashed (though none were detected in the regressions). > > > Diffs > - > > src/cpu/inorder/cpu.hh 7338bc628489 > src/cpu/inorder/cpu.cc 7338bc628489 > src/cpu/inorder/resource.cc 7338bc628489 > src/cpu/inorder/resource_pool.cc 7338bc628489 > > Diff: http://reviews.m5sim.org/r/369/diff > > > Testing > --- > > > Thanks, > > Steve > > ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: inorder: replace schedEvent() code with reschedule().
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/369/#review617 --- Ship it! My only question is does it work? it seems like the if statements don't translate exactly, but that could just be over-specifying the if (e.g. can you ever fall out with out any action?). - Ali On 2011-01-04 14:41:12, Steve Reinhardt wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/369/ > --- > > (Updated 2011-01-04 14:41:12) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > --- > > inorder: replace schedEvent() code with reschedule(). > There were several copies of similar functions that looked > like they all replicated reschedule(), so I replaced them > with direct calls. Keeping this separate from the previous > cset since there may be some subtle functional differences > if the code ever reschedules an event that is scheduled but > not squashed (though none were detected in the regressions). > > > Diffs > - > > src/cpu/inorder/cpu.hh 7338bc628489 > src/cpu/inorder/cpu.cc 7338bc628489 > src/cpu/inorder/resource.cc 7338bc628489 > src/cpu/inorder/resource_pool.cc 7338bc628489 > > Diff: http://reviews.m5sim.org/r/369/diff > > > Testing > --- > > > Thanks, > > Steve > > ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: inorder: replace schedEvent() code with reschedule().
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/369/#review599 --- Ship it! Looks good. I assume it works. - Nathan On 2011-01-04 14:41:12, Steve Reinhardt wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/369/ > --- > > (Updated 2011-01-04 14:41:12) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > --- > > inorder: replace schedEvent() code with reschedule(). > There were several copies of similar functions that looked > like they all replicated reschedule(), so I replaced them > with direct calls. Keeping this separate from the previous > cset since there may be some subtle functional differences > if the code ever reschedules an event that is scheduled but > not squashed (though none were detected in the regressions). > > > Diffs > - > > src/cpu/inorder/cpu.hh 7338bc628489 > src/cpu/inorder/cpu.cc 7338bc628489 > src/cpu/inorder/resource.cc 7338bc628489 > src/cpu/inorder/resource_pool.cc 7338bc628489 > > Diff: http://reviews.m5sim.org/r/369/diff > > > Testing > --- > > > Thanks, > > Steve > > ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
[m5-dev] Review Request: inorder: replace schedEvent() code with reschedule().
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/369/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- inorder: replace schedEvent() code with reschedule(). There were several copies of similar functions that looked like they all replicated reschedule(), so I replaced them with direct calls. Keeping this separate from the previous cset since there may be some subtle functional differences if the code ever reschedules an event that is scheduled but not squashed (though none were detected in the regressions). Diffs - src/cpu/inorder/cpu.hh 7338bc628489 src/cpu/inorder/cpu.cc 7338bc628489 src/cpu/inorder/resource.cc 7338bc628489 src/cpu/inorder/resource_pool.cc 7338bc628489 Diff: http://reviews.m5sim.org/r/369/diff Testing --- Thanks, Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
