> On June 11, 2013, 7:03 a.m., Steve Reinhardt wrote:
> > Same comments as before: (1) please append to rather than replace existing 
> > copyrights and (2) why not just merge this with the previous one.
> 
> Andreas Hansson wrote:
>     The replacing here was based on the fact that nothing of the original 
> file is left. Do you still think the existing copyright should stay even 
> though none of the contents did?
>     
>     I'll merge the two if desired. The main reason I did not do so initially 
> is that this one requires stats updates whereas the previous one does not.

Looking at the diffs, we still have two complete lines 'from m5.objects import 
*' and 'nb_cores = 4' left over from the original files (not to mention a 
common expression or two that is less than a full line).  I wouldn't say that's 
significant, but it seems to me that the ARM copyright has been added to files 
over equally minor amounts of code, so it gives a bad appearance for an ARM 
person in particular to not be just as pedantic when the shoe is on the other 
foot.  Also there's really no cost or harm to leaving the existing copyrights.

I'll leave the merging up to you; I don't see a gain in keeping them separate, 
and I think there is a slight loss in having closely related changes split 
across two patches, but it's not a big deal either way.


- Steve


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


On June 6, 2013, 2:33 p.m., Andreas Hansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1902/
> -----------------------------------------------------------
> 
> (Updated June 6, 2013, 2:33 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Description
> -------
> 
> Changeset 9758:47d9b6068c50
> ---------------------------
> config: Use BaseSESystem in multi-processor regressions
> 
> This patch adopts the BaseSESystem also in the o3-timing-mp,
> simple-timing-mp and simple-atomic-mp regressions (only SPARC).
> 
> The default builder is used with existing caches sizes as they match
> what was used in these scripts.
> 
> Due to the different order the ports are connected, the bus stats get
> shuffled around. A separate patch bumps the port indices. Besides
> this, all behaviour is exactly the same.
> 
> 
> Diffs
> -----
> 
>   tests/configs/o3-timing-mp.py 101441a7b420 
>   tests/configs/simple-atomic-mp.py 101441a7b420 
>   tests/configs/simple-timing-mp.py 101441a7b420 
> 
> Diff: http://reviews.gem5.org/r/1902/diff/
> 
> 
> Testing
> -------
> 
> All regressions pass
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

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

Reply via email to