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



configs/ruby/MESI_Three_Level.py (line 106)
<http://reviews.gem5.org/r/2776/#comment5361>

    Should these changes be in a different patch since they are orthogonal to 
the Ruby tester?
    
    I would be OK to keep them here, but you should update the commit message 
to say you change the clock domain for all of the controllers.



src/mem/ruby/system/RubyPort.cc (line 291)
<http://reviews.gem5.org/r/2776/#comment5362>

    I don't understand why the random tester doesn't want to retry failed 
requests. Could you explain why this feature is necessary? It's been a few 
years since I've looked deeply at the random tester. I'll hold off on 
broadcasting my opinion on this until I understand it better :).
    
    Again, this may be more appropriate as a separate patch, but I'm not going 
to push hard on this.



src/mem/ruby/system/RubyPort.cc (line 373)
<http://reviews.gem5.org/r/2776/#comment5360>

    Did you mean to call the new sendRetries function here?



src/mem/ruby/system/RubyPort.cc (line 400)
<http://reviews.gem5.org/r/2776/#comment5358>

    Does this function ever get called?
    
    Is this in one of the (many many) patches I haven't gotten to yet, or 
should be called from above?



src/mem/ruby/system/RubyPort.cc (line 406)
<http://reviews.gem5.org/r/2776/#comment5359>

    Does it make sense to call this function when the retryList is empty? Why 
not have an assert instead of an if statement which guards the entire function? 
    
    I suggest either changing the name (trySendRetries), or using the if 
statement outside of this function and instead use an assert within the 
function.


- Jason Power


On May 11, 2015, 10:28 p.m., Tony Gutierrez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/2776/
> -----------------------------------------------------------
> 
> (Updated May 11, 2015, 10:28 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 10833:e624796bae17
> ---------------------------
> ruby: cleaner ruby tester support
> 
> This patch allows the ruby random tester to use ruby ports that may only
> support instr or data requests.  This patch is similar to a previous changeset
> (8932:1b2c17565ac8) that was unfortunately broken by subsequent changesets.
> This current patch implements the support in a more straight-forward way.
> The patch also includes better DPRINTFs and generalizes the retry behavior
> needed by the ruby tester so that other testers/cpu models can use it as well.
> 
> 
> Diffs
> -----
> 
>   configs/example/ruby_random_test.py 
> fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   configs/ruby/MESI_Three_Level.py fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   configs/ruby/MESI_Two_Level.py fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   configs/ruby/MI_example.py fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   configs/ruby/MOESI_CMP_directory.py 
> fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   configs/ruby/MOESI_CMP_token.py fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   configs/ruby/MOESI_hammer.py fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   src/cpu/testers/rubytest/Check.cc fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   src/cpu/testers/rubytest/CheckTable.cc 
> fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   src/cpu/testers/rubytest/RubyTester.hh 
> fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   src/cpu/testers/rubytest/RubyTester.cc 
> fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   src/cpu/testers/rubytest/RubyTester.py 
> fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   src/mem/packet_queue.cc fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   src/mem/ruby/system/RubyPort.hh fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   src/mem/ruby/system/RubyPort.cc fbdaa08aaa426b9f4660c366f934ccb670d954ec 
>   src/mem/ruby/system/Sequencer.py fbdaa08aaa426b9f4660c366f934ccb670d954ec 
> 
> Diff: http://reviews.gem5.org/r/2776/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tony Gutierrez
> 
>

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

Reply via email to