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

[Wenlei He via Phabricator via llvm-commits]

wenlei 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:
> 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.
> 
A slight preference for this to be generic. The problem you're dealing with is the unordered nature of MD. Previously it works because there's the assumption that only one profile MD exists, which is not true for invoke.

It's cleaner and more robust if you simply make all APIs not depending on MD order. You don't need a hashmap, but how about just always search through MD when looking for branch_weights or VP. Specifically, that's changing the general implementation of extractBranchWeights and extractProfTotalWeight to be order independent regardless of the opcode. 


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

https://reviews.llvm.org/D133121