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

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/include/llvm/IR/Instructions.h:3973
+  // is not found.
+  static bool getProfMetaData(MDNode *MD, MDNode *&BranchWeightMD,
+                              MDNode *&IndirectCallMD) {
----------------
xur wrote:
> 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?
Sounds good for now.



================
Comment at: llvm/lib/IR/Metadata.cpp:1512
+    if (!ProfileData)
+      return false;
+  }
----------------
hoy wrote:
> This seems to change existing behavior for invokes from legacy IR that have only VP metadata.
How about this? Should both branch_weight and vp metadata be checked here to be downwards compatible? 


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

https://reviews.llvm.org/D133121