[PATCH] D121419: [WPD] Extend checking mode to support fallback to indirect call

Mingming Liu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 10 17:33:12 PST 2022


luna accepted this revision.
luna added a comment.
This revision is now accepted and ready to land.

It occurred to me that the WPD internal doc also mentions `--wholeprogramdevirt-check`, and current example uses it as a boolean.

Overall changing type of a debugging option makes sense (users would be able to figure out if they see an error, and there are enough time to update g3doc before a release)



================
Comment at: llvm/include/llvm/Transforms/Utils/CallPromotionUtils.h:84
+/// returned.
+CallBase &versionCallSite(CallBase &CB, Value *Callee, MDNode *BranchWeights);
+
----------------
nit pick

I'm wondering if it's more conventional to keep examples [1] in cpp or in the header (both are pretty readable IMO)

[1] https://github.com/llvm/llvm-project/blob/fc968bcba4d7e02390f1c1cba76a6a2cad1ae59f/llvm/lib/Transforms/Utils/CallPromotionUtils.cpp#L195-L281


================
Comment at: llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp:1174
+        CallBase &NewInst = versionCallSite(CB, Callee, Weights);
+        NewInst.setCalledOperand(Callee);
+      }
----------------
`llvm::promoteCall` [1] clears two types of metadata and then compares function type.

Function types are the same (for virtual functions); I'm wondering if it's needed to clear two types of metadata here (for my understanding)

[1] https://github.com/llvm/llvm-project/blob/fc968bcba4d7e02390f1c1cba76a6a2cad1ae59f/llvm/lib/Transforms/Utils/CallPromotionUtils.cpp#L456


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121419



More information about the llvm-commits mailing list