On Wed, 13 May 2015, Brad Beckmann wrote:
On May 12, 2015, 3:48 a.m., Nilay Vaish wrote:
This patch is incorrect on a couple of counts. We are not in 32-bit
world. Secondly, you should expose the getAddress() function as we
expose functions related to other classes.
This comment is dissappointing and unprofessional. I'm well aware of
address sizes supported in modern systems. Also I would like to point
out this patch adds back in a function verbatim that was previously
removed. I'm not introducing something new here.
As the description specifies, this funciton is used to convert an
address offset to an integer, not a 64-bit address. An integer is
needed because that is required to create a NodeID. Also there are
plenty of uses of 32-bit integers in this file and across gem5. I do
not think anyone has expectations to instantiate more than 2 B ruby
controllers.
I do not care how you are using the function. The input is a 64-bit value
which is being casted to 32-bits. You should at least check that the
input value is less than 2^32. Or else provide the right return type.
Even if in your protocols you are going to use only 32-bit values, the
function would still function correctly. Why do you want to assume that
who ever else will use the function would use a 32-bit value as input?
Finally what is the benefit of exposing the getAddress function and
doing the casting within SLICC? It is not how these sorts of
conversions have been handled in the past. There doesn't seem to be any
real benefit other than adding more work for us.
Firstly, I think using member functions is generally better than using
independent functions in terms of code organization and readability. Is
is not the same argument why we use C++ classes? Secondly, the person
authoring a protocol would be free to decide the type they want.
--
Nilay
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
