[PATCH] D128858: [llvm] Provide utility function for MD_prof

[Justin Bogner via Phabricator via llvm-commits]

bogner added a comment.

> There's an argument to be made that most of these utility functions should just be part of Metadata.cpp.

It's easy enough to merge these into Metadata.cpp later if we want to. I think a separate utils library is fine for now.



================
Comment at: llvm/include/llvm/IR/ProfDataUtils.h:42
+///
+/// \param ProfileData The profile data to extract the totla weight from
+/// \param TotalWeights input variable to fill with total weights
----------------
typo totla/total


================
Comment at: llvm/include/llvm/IR/ProfDataUtils.h:46
+/// \return false if no metadata was found.
+bool extractTotalBranchWeights(const MDNode *ProfileData,
+                               uint64_t &TotalWeights);
----------------
Would "extractRawBranchWeights" be a better name here? I guess this matches the naming of the user, but I think it's a bit hard to tell the difference between this and the extractBranchWeights functions from the name/doc comments as is.


================
Comment at: llvm/lib/IR/ProfDataUtils.cpp:50
+      Weights.clear();
+      return false;
+    }
----------------
Can we actually get here if we have valid metadata? This looks like something is malformed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128858