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

[Rong Xu via Phabricator via llvm-commits]

xur marked 2 inline comments as done.
xur added inline comments.


================
Comment at: llvm/include/llvm/IR/Instructions.h:3973
+  // is not found.
+  static bool getProfMetaData(MDNode *MD, MDNode *&BranchWeightMD,
+                              MDNode *&IndirectCallMD) {
----------------
hoy wrote:
> 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?
As of now, the branch weight metadata is only attached to terminator instructions in FDO. 
In sampleFDO, a branch weight can also be attached to a direct call.
VP metadata (more specifically, indirect call metadata) can only be with indirect-call or invoke instruction.
So in current code, we only have this situation for invoke instruction. This function can be written in a more generic way. But there is no use case for now.
I prefer keep this simple form and add a TODO (and expand when there is a use case). What do you think?


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


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

https://reviews.llvm.org/D133121