> On Jan. 13, 2016, 10:06 p.m., Andreas Hansson wrote:
> > src/mem/protocol/RubySlicc_Exports.sm, line 44
> > <http://reviews.gem5.org/r/3185/diff/5/?file=53114#file53114line44>
> >
> >     seems unrelated

These functions are modified an used by patch 3206.  Ideally this change 
should've been posted as apart of the dirty-bit-per-word support for ruby 
patch.  However, unless you strenously object, will check it in as is so that 
we can move on to higher priority tasks.


> On Jan. 13, 2016, 10:06 p.m., Andreas Hansson wrote:
> > src/mem/protocol/RubySlicc_Exports.sm, line 59
> > <http://reviews.gem5.org/r/3185/diff/5/?file=53114#file53114line59>
> >
> >     unrelated?

Sure we can remove this line.


> On Jan. 13, 2016, 10:06 p.m., Andreas Hansson wrote:
> > src/mem/protocol/RubySlicc_Exports.sm, line 170
> > <http://reviews.gem5.org/r/3185/diff/5/?file=53114#file53114line170>
> >
> >     This seems unrelated

Similar to above, these functions are modified an used by patch 3206.  Ideally 
this change should've been posted as apart of the dirty-bit-per-word support 
for ruby patch.  However, unless you strenously object, will check it in as is 
so that we can move on to higher priority tasks.


> On Jan. 13, 2016, 10:06 p.m., Andreas Hansson wrote:
> > src/mem/protocol/RubySlicc_Types.sm, line 129
> > <http://reviews.gem5.org/r/3185/diff/5/?file=53115#file53115line129>
> >
> >     seems orthogonal

Orthogonal?  These are used below in RubyReqeust.


> On Jan. 13, 2016, 10:06 p.m., Andreas Hansson wrote:
> > src/mem/request.hh, line 210
> > <http://reviews.gem5.org/r/3185/diff/5/?file=53116#file53116line210>
> >
> >     Where do these come from? Is there a spec somewhere? A pointer would be 
> > good.

Yes, http://hsafoundation.com.  I recommend not including the URL in the code 
though.  It just causes extra hassle when Legal teams scan the code.


> On Jan. 13, 2016, 10:06 p.m., Andreas Hansson wrote:
> > src/mem/request.hh, line 760
> > <http://reviews.gem5.org/r/3185/diff/5/?file=53116#file53116line760>
> >
> >     Given that these flags are all very specific and used by a single 
> > module, is it worth holding off on all the getters and simply rely on the 
> > getMemSpaceConfigFlags()?

Why push back on this?  These getters make our code easier to read and the cost 
is minimal to those who don't use them.


> On Jan. 13, 2016, 10:06 p.m., Andreas Hansson wrote:
> > src/mem/ruby/system/RubyPort.cc, line 239
> > <http://reviews.gem5.org/r/3185/diff/5/?file=53119#file53119line239>
> >
> >     This seems unrelated

Since you asked us to split the patch, there is not an obvious place to put the 
code.  We would prefer not to split it, since it is a very simple change.


> On Jan. 13, 2016, 10:06 p.m., Andreas Hansson wrote:
> > src/mem/ruby/system/RubyPort.cc, line 473
> > <http://reviews.gem5.org/r/3185/diff/5/?file=53119#file53119line473>
> >
> >     unrelated

Again this small change had to be placed somewhere and there was not an obvious 
place to put it after you asked us to split the code.


- Brad


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


On Jan. 13, 2016, 9:24 p.m., Tony Gutierrez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3185/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2016, 9:24 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 11279:e858f2256ccb
> ---------------------------
> mem: misc flags for AMD gpu model
> 
> This patch add support to mark memory requests/packets with attributes defined
> in HSA, such as memory order and scope.
> 
> 
> Diffs
> -----
> 
>   src/mem/ruby/common/DataBlock.hh d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/slicc_interface/RubyRequest.hh 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/system/RubyPort.cc d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/protocol/RubySlicc_Exports.sm 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/protocol/RubySlicc_Types.sm 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/request.hh d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
> 
> Diff: http://reviews.gem5.org/r/3185/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tony Gutierrez
> 
>

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

Reply via email to