> On May 30, 2012, 10:18 a.m., Nathan Binkert wrote:
> > Interesting.  I'd click ship it, but it seems that maybe a little bit more 
> > thought on how we can make the checkpoint version number stuff actually 
> > work in practice would be good.

Yes, it does have the issue that version numbers don't automatically increment, 
but even if they did help would be required since someone has to write the 
migration function. I've thought about the various ways to auto-generate it and 
none of them work. The only solution is a regression test for checkpoints and 
if you update the regression you need to increment the version number. At least 
having the version information there lets you do something. Right now there is 
nothing at all to distinguish one from another so there is no hope of migrating.


> On May 30, 2012, 10:18 a.m., Nathan Binkert wrote:
> > src/sim/serialize.hh, line 60
> > <http://reviews.gem5.org/r/1238/diff/1/?file=27135#file27135line60>
> >
> >     It seems that this could easily get out of sync.   Any ideas how to 
> > prevent that from happening?  Be cool if we could somehow hash the various 
> > checkpoint functions.  Should we do the versioning on a per-object basis?  
> > That way a change in one isa wouldn't affect another, and it might be more 
> > likely that someone notices the need to update the version number for an 
> > object checkpoint.

I think that that is more complicated than needed. If a certain revision 
doesn't need any changes the code as written will just skip it. I think the 
only way that people are going to notice is with a checkpoint regression. While 
the restoring is a bit hard to do with our framework, the actually generation 
of the m5.cpt file shouldn't be too bad. 


> On May 30, 2012, 10:18 a.m., Nathan Binkert wrote:
> > src/sim/root.cc, line 135
> > <http://reviews.gem5.org/r/1238/diff/1/?file=27134#file27134line135>
> >
> >     Since this is only ever used here, you could simply do this and avoid 
> > the change to config/the_isa.hh:
> >     
> >     #define __STRINGIFY(X) #X
> >     std::string isa = __STRINGIFY(TheISA);
> >     #undef __STRINGIFY
> >

I'm happy to change this, but it seems like trading a auto-generated value 
which doesn't hurt anything but is everywhere for a bit if isolated ugliness. I 
guess I lean slightly toward the way it's currently done. 


- Ali


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/1238/#review2858
-----------------------------------------------------------


On May 30, 2012, 10:01 a.m., Ali Saidi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1238/
> -----------------------------------------------------------
> 
> (Updated May 30, 2012, 10:01 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Description
> -------
> 
> Changeset 9041:1f633efe95fe
> ---------------------------
> sim: Provide a framework for detecting out of data checkpoints and migrating 
> them.
> 
> 
> Diffs
> -----
> 
>   src/SConscript 8b9f227b64d8 
>   src/sim/root.hh 8b9f227b64d8 
>   src/sim/root.cc 8b9f227b64d8 
>   src/sim/serialize.hh 8b9f227b64d8 
>   util/cpt_upgrader.py PRE-CREATION 
> 
> Diff: http://reviews.gem5.org/r/1238/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ali Saidi
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to