[PATCH] D73221: [PGO] Attach appropriate funclet operand bundles to value profiling instrumentation calls

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

Code makes sense and tests look good, but I have a suggestion for how to make it simpler.



================
Comment at: llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp:844-848
+  // If there are memintrinsic operands to be instrumented and the function
+  // contains exception handling blocks that require funclet operand bundles,
+  // then it is possible that a funclet value will need to be set on the
+  // instrumentation call. The memintrisic calls are not marked with funclet operand
+  // bundles, so this needs to be computed.
----------------
The underlying invariant is that non-intrinsic calls will carry funclet bundles. Intrinsic calls do not. Rather than special casing memop size instrumentation, I think it would be better to:
- always calculate colors
- below, use the funclet bundle directly when instrumenting non-intrinsic calls
- use the color map when instrumenting intrinsic calls (memops)

This logic will be less fragile in the face of future value profiling kinds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73221