[PATCH] D128860: [llvm][NFC] Refactor code to use ProfDataUtils

[Paul Kirth via Phabricator via llvm-commits]

paulkirth added inline comments.


================
Comment at: llvm/include/llvm/IR/Instruction.h:356
   /// Returns false if no metadata or invalid metadata was found.
   bool extractProfMetadata(uint64_t &TrueVal, uint64_t &FalseVal) const;
 
----------------
bogner wrote:
> There's an argument for removing this entirely and updating the users to just use extractBranchWeights directly (possibly with a specialization of that function that makes the True/False case convenient). I'm not sure how strongly I feel about it, but all of the branch weight stuff being in one place seems kind of nice. WDYT?
I think putting all the branch weights utilities in one place is a good idea. Replacing these users was something I was also thinking about, so maybe that's the way we should do it here. 

I'll take a look at the users and see if there's a straightforward way to migrate them or if a specialization makes more sense.


================
Comment at: llvm/include/llvm/IR/Instruction.h:361
+  /// Returns false if no metadata or invalid metadata was found.
+  bool extractProfMetadata(SmallVector<uint32_t> &Weights) const;
+
----------------
bogner wrote:
> Is this used anywhere? We can probably omit it until it is.
Yeah, I was hesitant about adding the interface, but wanted to provide an option for switch statements. Since there's currently no users I'll remove the interface. Like you said once someone wants/needs it, It's really easy to add back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128860