[PATCH] D137149: Use PassGate from LLVMContext if any otherwise global one

Arthur Eubanks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 15 14:26:11 PST 2022


aeubanks added a comment.

In D137149#3914097 <https://reviews.llvm.org/D137149#3914097>, @ebrevnov wrote:

> In D137149#3913303 <https://reviews.llvm.org/D137149#3913303>, @aeubanks wrote:
>
>> chasing pointers can be expensive, and the current compile time regressions are unacceptable so requesting changes in phabricator
>>
>> perhaps we can only register the callback if either `-opt-bisect-limit` is set or if some other new `cl::opt` (`-force-pass-gate-instrumentation`?), then you'd specify that option when debugging?
>
> That looks a really hacky to me ... I would choose that option as a last resort.
> Once realized we can't afford requesting pass gate from the module on each invocation of callback (due to CT) I'm inclined to think that passing LLVMContext (and FAM as well) during construction of StandardInstrumentations is the right way to go. This way we explicitly set expectations that StandardInstrumentations is tied to a single LLVMContext. Are you OK with such approach? If not then I will go with the caching approach (but I think it's less expressive and more complicated).

I think I'd stlil slightly prefer caching (could cache the results of `PassGate.isEnabled` instead), but I think I'd be ok with adding LLVMContext as a parameter. We should add it as a required parameter to the `StandardInstrumentations` constructor though so people don't forget about it. (we should also move the `FAM` parameter into the constructor, but that's a separate issue)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137149



More information about the llvm-commits mailing list