[PATCH] D79961: [PGO] Fix computation of fuction Hash

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

I worry that the hash version bump isn't complete. Doesn't the hash version used need to be read/written with the profile data file somewhere?



================
Comment at: clang/lib/CodeGen/CodeGenPGO.cpp:148
     return PGO_HASH_V1;
   return PGO_HASH_V2;
 }
----------------
I guess this needs an update now?


================
Comment at: clang/lib/CodeGen/CodeGenPGO.cpp:292
         return PGOHash::BinaryOperatorLOr;
       if (HashVersion == PGO_HASH_V2) {
         switch (BO->getOpcode()) {
----------------
Should this be >= PGO_HASH_V2 now? And similarly below?


================
Comment at: clang/lib/CodeGen/CodeGenPGO.cpp:754
+    // is buggy because it converts a uint64_t into an array of uint8_t.
+    if (HashVersion == PGO_HASH_V1 or HashVersion == PGO_HASH_V2) {
+      MD5.update({(uint8_t)Working});
----------------
Maybe just "HashVersion < PGO_HASH_V3" would be simpler?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79961/new/

https://reviews.llvm.org/D79961