Should this not be folded into the original patch?

src/mem/multi_level_page_table_impl.hh (Diff revision 1)

I'm not sure I'm a massive fan...but then again, I don't quite know what to suggest instead.

Show all issues

1. Steve Reinhardt Sept. 30, 2014, 1:51 p.m. (Sept. 30, 2014, 1:51 p.m.)

   I think reformatting into two lines would be a win:
   
     for (; size > 0;
          size -= pageSize, vaddr += pageSize, new_vaddr += pageSize) {
   
   The other alternative would be to make a more 'normal' for loop out of it:
   
     for (; size > 0; size -= pageSize) {
       [...]
       vaddr += pageSize;
       new_vaddr += pageSize;
     }
   
   but while that's more conventional I don't really find it more readable; combining the loop-related updates as it currently stands is nice in my opinion.

2. Alexandru Dutu Sept. 30, 2014, 2:09 p.m. (Sept. 30, 2014, 2:09 p.m.)

   Having a for statement on 3 lines was a bit strange for me as well, however I find it a bit more readable. So I will go with Steve's suggestion to reformat it in 2 lines.

src/mem/multi_level_page_table_impl.hh (Diff revision 2)

i - 1

Show all issues

src/mem/page_table.cc (Diff revision 2)

Same here as the other

Show all issues

Some very minor things. No need to re-post once they are fixed.