Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable pass timings reporting with NewPassManager #1163

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Enable pass timings reporting with NewPassManager #1163

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
08a9a4b
Export PIC as separate class
yashssh Feb 26, 2025
4f21d54
Updated PIC apis and export pass timings reporting in new pass manager
yashssh Feb 26, 2025
90fe538
formatting and minor fixes
yashssh Feb 27, 2025
ee3b12a
Better argument names
yashssh Feb 28, 2025
fd81b94
Fix typo in version check
yashssh Feb 28, 2025
35da642
Don't need to expose PIC in python
yashssh Feb 28, 2025
86255fe
Change API names addressing review comments
yashssh Mar 3, 2025
395a996
Prevent re-enabling of pass timings
yashssh Mar 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 45 additions & 7 deletions ffi/newpassmanagers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ typedef OpaquePipelineTuningOptions *LLVMPipelineTuningOptionsRef;
DEFINE_SIMPLE_CONVERSION_FUNCTIONS(PipelineTuningOptions,
LLVMPipelineTuningOptionsRef)

struct OpaqueTimePassesHandler;
typedef OpaqueTimePassesHandler *LLVMTimePassesHandlerRef;
DEFINE_SIMPLE_CONVERSION_FUNCTIONS(TimePassesHandler, LLVMTimePassesHandlerRef)

static TargetMachine *unwrap(LLVMTargetMachineRef P) {
return reinterpret_cast<TargetMachine *>(P);
}
Expand Down Expand Up @@ -281,21 +285,55 @@ LLVMPY_DisposePipelineTuningOptions(LLVMPipelineTuningOptionsRef PTO) {

// PB

API_EXPORT(LLVMTimePassesHandlerRef)
LLVMPY_CreateLLVMTimePassesHandler() {
return llvm::wrap(new TimePassesHandler(true));
}

API_EXPORT(void)
LLVMPY_DisposeLLVMTimePassesHandler(LLVMTimePassesHandlerRef TimePassesRef) {
delete llvm::unwrap(TimePassesRef);
}

API_EXPORT(void)
LLVMPY_SetTimePassesNPM(LLVMPassBuilderRef PBRef, LLVMTimePassesHandlerRef TimePassesRef) {
TimePassesHandler *TP = llvm::unwrap(TimePassesRef);
TimePassesIsEnabled = true;
PassBuilder *PB = llvm::unwrap(PBRef);
PassInstrumentationCallbacks *PIC = PB->getPassInstrumentationCallbacks();
TP->registerCallbacks(*PIC);
}

API_EXPORT(void)
LLVMPY_ReportAndResetTimingsNPM(LLVMTimePassesHandlerRef TimePassesRef,
const char **outmsg) {
std::string osbuf;
raw_string_ostream os(osbuf);
TimePassesHandler *TP = llvm::unwrap(TimePassesRef);
TP->setOutStream(os);
TP->print();
os.flush();
*outmsg = LLVMPY_CreateString(os.str().c_str());
TimePassesIsEnabled = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some asymmetry here - do pass instrumentation callbacks need to be unregistered if we've disabled pass timing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. A unique PICallbacks object belongs to PassBuilder object and is used to do stuff like this(hooks to enable/disable pass timings) after the optimisation pipeline has been built.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you start and finish pass timing, and then start pass timing again, do you end up with the callbacks registered twice though?

Copy link
Contributor Author

@yashssh yashssh Mar 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TimePassesHandler::registerCallbacks has an early exit which will prevent this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how that early exist handler helps in this scenario, in which the user at the Python level has written

pb.start_pass_timing()
pb.finish_pass_timing()
pb.start_pass_timing()

At this point pass timing is re-enabled, and it looks like the callbacks will be registered a second time at https://github.com/numba/llvmlite/pull/1163/files#diff-45eef2a2ab57512c9c03ab44fbebc4228722e001d790af044bd2ca511df08414R306

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah you are right. Sorry, I read the code wrong.
Right, I tried doing this locally

pb.start_pass_timing()
mpm = pb.getModulePassManager()
mpm.run(module, pb)
report = pb.finish_pass_timing()

pb.start_pass_timing()
mpm.run(module, pb)
report2 = pb.finish_pass_timing()

I get an asserting failure from LLVM:

python: /home/yashwants/work/upstream/numba-llvm16/llvm/lib/IR/PassTimingInfo.cpp:255: void llvm::TimePassesHandler::startPassTimer(StringRef): Assertion `!ActivePassTimer && "should only have one pass timer at a time"' failed.

Which is also consistent with the fact we allow running the optimisation pipeline with the PassManger object only once. Should we try to handle this in Python? Imo we can ignore this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't have CPython get killed by a user's Python coding error. We could have a check in start_pass_timing that checks if it's already been called and raises a RuntimeError if it has already been called before, to protect the interpreter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I have added a mechanism to prevent enabling time passes multiple times, what do you think?

}

API_EXPORT(LLVMPassBuilderRef)
LLVMPY_CreatePassBuilder(LLVMTargetMachineRef TM,
LLVMPipelineTuningOptionsRef PTO) {
TargetMachine *target = llvm::unwrap(TM);
PipelineTuningOptions *pt = llvm::unwrap(PTO);
LLVMPY_CreatePassBuilder(LLVMTargetMachineRef TMRef,
LLVMPipelineTuningOptionsRef PTORef) {
TargetMachine *TM = llvm::unwrap(TMRef);
PipelineTuningOptions *PTO = llvm::unwrap(PTORef);
PassInstrumentationCallbacks *PIC = new PassInstrumentationCallbacks();
#if LLVM_VERSION_MAJOR < 16
return llvm::wrap(new PassBuilder(target, *pt, None, PIC));
return llvm::wrap(new PassBuilder(TM, *PTO, None, PIC));
#else
return llvm::wrap(new PassBuilder(target, *pt, std::nullopt, PIC));
return llvm::wrap(new PassBuilder(TM, *PTO, std::nullopt, PIC));
#endif
}

API_EXPORT(void)
LLVMPY_DisposePassBuilder(LLVMPassBuilderRef PB) { delete llvm::unwrap(PB); }
LLVMPY_DisposePassBuilder(LLVMPassBuilderRef PBRef) {
delete llvm::unwrap(PBRef);
}

static OptimizationLevel mapLevel(int speed_level, int size_level) {
switch (size_level) {
Expand Down
2 changes: 1 addition & 1 deletion llvmlite/binding/ffi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def _make_opaque_ref(name):
LLVMSectionIteratorRef = _make_opaque_ref("LLVMSectionIterator")
LLVMOrcLLJITRef = _make_opaque_ref("LLVMOrcLLJITRef")
LLVMOrcDylibTrackerRef = _make_opaque_ref("LLVMOrcDylibTrackerRef")

LLVMTimePassesHandlerRef = _make_opaque_ref("LLVMTimePassesHandler")
LLVMPipelineTuningOptionsRef = _make_opaque_ref("LLVMPipeLineTuningOptions")
LLVMModulePassManagerRef = _make_opaque_ref("LLVMModulePassManager")
LLVMFunctionPassManagerRef = _make_opaque_ref("LLVMFunctionPassManager")
Expand Down
49 changes: 46 additions & 3 deletions llvmlite/binding/newpassmanagers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from ctypes import c_bool, c_int, c_size_t
from ctypes import c_bool, c_int, c_size_t, c_char_p, POINTER
from enum import IntFlag
from llvmlite.binding import ffi

Expand Down Expand Up @@ -209,11 +209,18 @@ def _dispose(self):


class PassBuilder(ffi.ObjectRef):
class TimePassesHandler(ffi.ObjectRef):
def __init__(self):
super().__init__(ffi.lib.LLVMPY_CreateLLVMTimePassesHandler())

def _dispose(self):
ffi.lib.LLVMPY_DisposeLLVMTimePassesHandler(self)

def __init__(self, tm, pto):
super().__init__(ffi.lib.LLVMPY_CreatePassBuilder(tm, pto))
self._pto = pto
self._tm = tm
self._time_passes = self.TimePassesHandler()

def getModulePassManager(self):
return ModulePassManager(
Expand All @@ -227,6 +234,24 @@ def getFunctionPassManager(self):
self, self._pto.speed_level, self._pto.size_level)
)

def set_time_passes(self):
"""Enable the pass timers.
"""
ffi.lib.LLVMPY_SetTimePassesNPM(self, self._time_passes)

def report_and_reset_timings(self):
"""Returns the pass timings report and resets the LLVM internal timers.
Pass timers are enabled by ``set_time_passes()``. If the timers are not
enabled, this function will return an empty string.
Returns
-------
res : str
LLVM generated timing report.
"""
with ffi.OutputString() as buf:
ffi.lib.LLVMPY_ReportAndResetTimingsNPM(self._time_passes, buf)
return str(buf)

def _dispose(self):
ffi.lib.LLVMPY_DisposePassBuilder(self)

Expand Down Expand Up @@ -339,11 +364,29 @@ def _dispose(self):
# PassBuilder

ffi.lib.LLVMPY_CreatePassBuilder.restype = ffi.LLVMPassBuilderRef
ffi.lib.LLVMPY_CreatePassBuilder.argtypes = [ffi.LLVMTargetMachineRef,
ffi.LLVMPipelineTuningOptionsRef,]
ffi.lib.LLVMPY_CreatePassBuilder.argtypes = [
ffi.LLVMTargetMachineRef,
ffi.LLVMPipelineTuningOptionsRef,
]

ffi.lib.LLVMPY_DisposePassBuilder.argtypes = [ffi.LLVMPassBuilderRef,]

ffi.lib.LLVMPY_CreateLLVMTimePassesHandler.restype = \
ffi.LLVMTimePassesHandlerRef

ffi.lib.LLVMPY_DisposeLLVMTimePassesHandler.argtypes = [
ffi.LLVMTimePassesHandlerRef,]

ffi.lib.LLVMPY_SetTimePassesNPM.argtypes = [
ffi.LLVMPassBuilderRef,
ffi.LLVMTimePassesHandlerRef,
]

ffi.lib.LLVMPY_ReportAndResetTimingsNPM.argtypes = [
ffi.LLVMTimePassesHandlerRef,
POINTER(c_char_p),
]

# Pipeline builders

ffi.lib.LLVMPY_buildPerModuleDefaultPipeline.restype = \
Expand Down
29 changes: 29 additions & 0 deletions llvmlite/tests/test_binding.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3066,6 +3066,35 @@ def test_get_function_pass_manager(self):
fpm.run(self.module().get_function("sum"), pb)
pb.close()

def test_time_passes(self):
"""Test pass timing reports for O3 and O0 optimization levels"""
def run_with_timing(speed_level):
mod = self.module()
pb = self.pb(speed_level=speed_level, size_level=0)
pb.set_time_passes()
mpm = pb.getModulePassManager()
mpm.run(mod, pb)
report = pb.report_and_reset_timings()
pb.close()
return report

report_O3 = run_with_timing(3)
report_O0 = run_with_timing(0)

self.assertIsInstance(report_O3, str)
self.assertIsInstance(report_O0, str)
self.assertEqual(report_O3.count("Pass execution timing report"), 1)
self.assertEqual(report_O0.count("Pass execution timing report"), 1)

def test_empty_report(self):
mod = self.module()
pb = self.pb()
mpm = pb.getModulePassManager()
mpm.run(mod, pb)
report = pb.report_and_reset_timings()
pb.close()
self.assertFalse(report)


class TestNewModulePassManager(BaseTest, NewPassManagerMixin):
def pm(self):
Expand Down
Loading