[PATCH] D156389: [BOLT] Add support for instrumenting conditional tail calls

[Maksim Panchenko via Phabricator via llvm-commits]

maksfb added inline comments.


================
Comment at: bolt/lib/Passes/Instrumentation.cpp:103-104
                                             const BinaryFunction &ToFunction,
-                                            uint32_t To, bool IsInvoke) {
+                                            uint32_t To, bool IsInvoke,
+                                            bool IsCondTailCall) {
   CallDescription CD;
----------------
Can we merge `IsInvoke` and `IsCondTailCall` into a single argument, e.g. `NeedsInstrumentation`?


================
Comment at: bolt/lib/Passes/Instrumentation.cpp:231-232
     BinaryBasicBlock &FromBB, uint32_t From, BinaryFunction &ToFunc,
     BinaryBasicBlock *TargetBB, uint32_t ToOffset, bool IsLeaf, bool IsInvoke,
-    FunctionDescription *FuncDesc, uint32_t FromNodeID, uint32_t ToNodeID) {
+    bool IsCondTailCall, FunctionDescription *FuncDesc, uint32_t FromNodeID,
+    uint32_t ToNodeID) {
----------------
Same args can me merged here too.


================
Comment at: bolt/lib/Passes/Instrumentation.cpp:396-397
+      bool IsCondTailCall = false;
+      // Identify conditional tail call by their offset being equal to parent
+      // blocks' input offset
+      if (TargetBB && TargetBB != &BB &&
----------------
While this approach works, a more explicit way to identify CTCs will be to mark TC instructions created from CTCs with a proper annotation and check for this annotation during instrumentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156389