> On April 17, 2012, 7:56 a.m., Andreas Hansson wrote:
> >
> 
> Andreas Hansson wrote:
>     I noticed this was pushed without fixing the copy paste error in the 
> description, and without any comments describing the issue of a single vs. 
> multiple memories. At the the first bit should be fixed.
> 
> Nilay Vaish wrote:
>     Andreas, I committed the patches. I am not too concerned about the commit
>     message. I agree there is repetition in it. But I don't think this 
> requires
>     to be fixed as the message is not describing some thing incorrectly, or 
>     missing out on some thing that it should have described. Secondly, this
>     patch is not concerned about multiple memories / disk images. I don't see
>     why the commit message should mention it at all.

I am not talking about the committ message, but rather my initial review 
stating that the message associated with the mem_size parameter is copy pasted 
from the disk image and not changed. I think it would make sense to have an 
accurate help message for the parameter, and thus would like to see it changed. 
Makes sense? I remarked upon this in my very first set of comments.

With respect to the multiple memories (or lack there of), I am merely 
requesting a comment line above the parameter stating that this assumes a 
single memory in e system and that for multiple memories the script itself has 
to be changed. I hope that also makes sense.


- Andreas


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


On April 16, 2012, 11:53 a.m., Jayneel Gandhi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/1150/
> -----------------------------------------------------------
> 
> (Updated April 16, 2012, 11:53 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Description
> -------
> 
> Added the options to Options.py for FS mode with backward compatibility. It 
> is good to provide an option to specify the disk image and the memory size 
> from command line since a lot of disk images are created to support different 
> benchmark suites as well as per user needs. Change in program also leads to 
> change in memory requirements. These options provide the interface to provide 
> both disk image and memory size from the command line and gives more 
> flexibility. 
> 
> 
> Diffs
> -----
> 
>   configs/common/Options.py a6830d615effc2e644fd3ebd2553c569c1d4607f 
>   configs/example/fs.py a6830d615effc2e644fd3ebd2553c569c1d4607f 
>   configs/example/ruby_fs.py a6830d615effc2e644fd3ebd2553c569c1d4607f 
> 
> Diff: http://reviews.gem5.org/r/1150/diff/
> 
> 
> Testing
> -------
> 
> Testing done to confirm backward compatibility
> 
> 
> Thanks,
> 
> Jayneel Gandhi
> 
>

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

Reply via email to