[GitHub] larroy commented on a change in pull request #10746: [MXNET-368] Fix mxnet::Context hash for 32 bit architectures, where size_t is 32 …

[GitBox <gi...@apache.org>]

larroy commented on a change in pull request #10746: [MXNET-368] Fix mxnet::Context hash for 32 bit architectures, where size_t is 32 …
URL: https://github.com/apache/incubator-mxnet/pull/10746#discussion_r185059619
 
 

 ##########
 File path: include/mxnet/base.h
 ##########
 @@ -365,7 +365,9 @@ constexpr size_t kMKLDNNAlign = 64;
 namespace std {
 template<> struct hash<mxnet::Context> {
   size_t operator()(const mxnet::Context& ctx) const {
-    return (static_cast<size_t>(ctx.dev_type) << 32) | ctx.dev_id;
+    size_t dev_type = static_cast<size_t>(ctx.dev_type);
+    size_t dev_id = static_cast<size_t>(ctx.dev_id);
+    return (dev_id << 3) ^ dev_type ^ (dev_id >> 29);
 
 Review comment:
   the previous one just discards the dev_type in 32 bit architectures when size_t is 32 bit. 
   
   I'm not claiming that is a good hash function, just not discarding dev_type.  
   
   As far as I know, this function doesn't need to be a "good hash" nor cryptographic hash. But the previous function didn't fulfill some of the requirements for this operator:
   
   http://en.cppreference.com/w/cpp/utility/hash

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services