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

Mon Nov 7 01:56:34 PST 2022

ebrevnov added a comment.

In D137149#3908943 <https://reviews.llvm.org/D137149#3908943>, @aeubanks wrote:

> In D137149#3907610 <https://reviews.llvm.org/D137149#3907610>, @ebrevnov wrote:
>
>> In D137149#3906302 <https://reviews.llvm.org/D137149#3906302>, @aeubanks wrote:
>>
>>> thanks for the context, agreed that using the LLVMContext's pass gate is what's intended
>>>
>>> the cleanest way IMO is to get the IR's LLVMContext (e.g. via `unwrapModule(IR, /*Force*/ true).getContext()`) and use the pass gate from there, rather than collect the pass gate at the beginning. WDYT of something like https://reviews.llvm.org/D137358?
>>
>> Totally agree. Just didn't realize we already can access the module. Integrated suggested approach except caching part. I think caching is unnecessary complication in this case.
>
> then you start running into compile time issues: https://llvm-compile-time-tracker.com/compare.php?from=66b830889d9bc4bfc58aa1ea47d1b3a7cbee7c5c&to=b7972184a59c0aa0b634de653be1c1363e980f4c&stat=instructions:u
> previously we didn't call `OptPassGateInstrumentation::registerCallbacks` and run these callbacks before every pass if `-opt-bisect-limit` was enabled, with this patch we do.

That's kind of unexpected (at least for me). Do we known why such lightweight operation noticeably impacts compile time? Is it getting a module from a loop or something else?

> caching mitigates a lot of that.

It does but it opens legality concerns. Do we have a guarantee that a module passed to the callback (through IR parameter) hasn't been changed between the invocations? If it can't change then why passing it to each callback invocation (as opposite to setting it once at creation time or later)? Same concerns apply to immutability of OptPassGate itself and it's IsEnabled field.

> but even better is if you can somehow go back to not calling `OptPassGateInstrumentation::registerCallbacks` when we're not using opt-bisect. With your per-LLVMContext opt-bisect, do you still respect `-opt-bisect-limit`, or > are you manually setting pass gates?

Currently I'm setting it manually/programmatically to LLVMContext which has preference over global on if set.

> if you're still using `-opt-bisect-limit`, we can go back to checking if it's set and skipping `OptPassGateInstrumentation::registerCallbacks` and we don't have to worry about caching.

In theory, we can introduce another global which will be set to 'true' only if any pass gate in the system is enabled and use that as a filter..... but that looks overcomplication I would really like to avoid.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137149