[PATCH] D133121: [PGO] Annotate branch_weight for invoke instruction

Hongtao Yu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 1 15:53:50 PDT 2022


hoy added inline comments.


================
Comment at: llvm/include/llvm/IR/Instructions.h:3969
+  // Find the "branch_weights" metadata and indirect-call metadata.
+  // Return true if either metadat is found.
+  // Return false if none of the metadtata is found.
----------------
nit: metadat -> metadata


================
Comment at: llvm/include/llvm/IR/Instructions.h:3973
+  // is not found.
+  static bool getProfMetaData(MDNode *MD, MDNode *&BranchWeightMD,
+                              MDNode *&IndirectCallMD) {
----------------
I'm wondering if this function can be made more generic and not restrict to invoke instruction only. I feel it's likely in the future that more types of metadata will be allowed for an instruction. How about returning a something like a hash map with the keys being the type of metadata?


================
Comment at: llvm/include/llvm/IR/Instructions.h:4011
+    if (!getProfMetaData(MD, BranchWeightMD, IndirectCallMD))
+      return nullptr;
+    return BranchWeightMD;
----------------
nit: the `return nullptr` isn't necessary as `BranchWeightMD` will be set to null if the branch metadata isn't there? Similarly below.


================
Comment at: llvm/lib/IR/Metadata.cpp:1512
+    if (!ProfileData)
+      return false;
+  }
----------------
This seems to change existing behavior for invokes from legacy IR that have only VP metadata.


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

https://reviews.llvm.org/D133121



More information about the llvm-commits mailing list