src/sim/futex_map.hh (Diff revision 1)

This is not the license we should be using. See the gpu-compute code for our license.

Show all issues

src/sim/futex_map.hh (Diff revision 1)

This comment needs more detail.

Show all issues

src/sim/futex_map.hh (Diff revision 1)

Newline after the return type.

Show all issues

src/sim/se_signal.cc (Diff revision 1)

Should this not be 2014, 2016?

Show all issues

src/sim/futex_map.hh (Diff revision 2)

I'm not sure if this is needed.

Show all issues

src/sim/futex_map.hh (Diff revision 2)

Why not something simple, like:
return std::hash(addr) ^ (std::hash(tgid) << 1)

Show all issues

1. Brandon Potter Nov. 14, 2016, 1:30 p.m. (Nov. 14, 2016, 1:30 p.m.)

   The return value has to be size_t which from the C standard will at least have a max value of 65536 (http://stackoverflow.com/questions/918787/whats-sizeofsize-t-on-32-bit-vs-the-various-64-bit-data-models). The key values are uint64_t. Since the value is cap starts at 65536, I wanted to make sure that the entire number was being fed into the hash. That's why I work through with the for loop that breaks uint64_t apart based on sizeof(size_t).
   I chose the largest prime number less than the max to start as the hash value. It could've been anything though; the choice was arbitrary.
   So, is this OK? This functor is really just a dumb widget to satisfy std::hash. I don't expect the futex_map to grow to be so large that the hash value matters much.

2. Michael LeBeane Nov. 14, 2016, 1:37 p.m. (Nov. 14, 2016, 1:37 p.m.)

   Yeah, I don't think the hash matters to much, which is why I prefer something simple.  Don't care too much though, do what you prefer.

src/sim/system.hh (Diff revision 2)

spurious whitespace change

Show all issues

1. Brandon Potter Nov. 14, 2016, 1:30 p.m. (Nov. 14, 2016, 1:30 p.m.)

   The include style checker does this on it's own. I can't qref the patch without this being done (or hacking on it a bit).

Ship It!

Ship It!