-
Notifications
You must be signed in to change notification settings - Fork 335
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
08a9a4b
4f21d54
90fe538
ee3b12a
fd81b94
35da642
86255fe
395a996
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||||||||
|
||||||||
|
@@ -208,12 +208,21 @@ def _dispose(self): | |||||||
ffi.lib.LLVMPY_DisposePipelineTuningOptions(self) | ||||||||
|
||||||||
|
||||||||
class TimePassesHandler(ffi.ObjectRef): | ||||||||
def __init__(self): | ||||||||
super().__init__(ffi.lib.LLVMPY_CreateTimePassesHandler()) | ||||||||
|
||||||||
def _dispose(self): | ||||||||
ffi.lib.LLVMPY_DisposeTimePassesHandler(self) | ||||||||
|
||||||||
|
||||||||
class PassBuilder(ffi.ObjectRef): | ||||||||
|
||||||||
def __init__(self, tm, pto): | ||||||||
super().__init__(ffi.lib.LLVMPY_CreatePassBuilder(tm, pto)) | ||||||||
self._pto = pto | ||||||||
self._tm = tm | ||||||||
self._time_passes_handler = None | ||||||||
|
||||||||
def getModulePassManager(self): | ||||||||
return ModulePassManager( | ||||||||
|
@@ -227,6 +236,39 @@ def getFunctionPassManager(self): | |||||||
self, self._pto.speed_level, self._pto.size_level) | ||||||||
) | ||||||||
|
||||||||
def start_pass_timing(self): | ||||||||
"""Enable the pass timers. | ||||||||
|
||||||||
Raises | ||||||||
------ | ||||||||
RuntimeError | ||||||||
If pass timing is already enabled. | ||||||||
""" | ||||||||
if self._time_passes_handler: | ||||||||
raise RuntimeError("Pass builder should only have one \ | ||||||||
pass timer at a time") | ||||||||
self._time_passes_handler = TimePassesHandler() | ||||||||
Comment on lines
+248
to
+249
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is a little clearer (the current message implies there's a way to remove a pass timer, which I think there isn't):
Suggested change
|
||||||||
ffi.lib.LLVMPY_EnableTimePasses(self, self._time_passes_handler) | ||||||||
|
||||||||
def finish_pass_timing(self): | ||||||||
"""Returns the pass timings report and disables the LLVM internal | ||||||||
timers. Pass timers are enabled by ``start_pass_timing()``. If the | ||||||||
timers are not enabled, this function will return an empty string. | ||||||||
|
||||||||
Returns | ||||||||
------- | ||||||||
res : str | ||||||||
LLVM generated timing report. | ||||||||
""" | ||||||||
|
||||||||
if not self._time_passes_handler: | ||||||||
raise RuntimeError("Pass timing is not enabled") | ||||||||
|
||||||||
with ffi.OutputString() as buf: | ||||||||
ffi.lib.LLVMPY_ReportAndDisableTimePasses( | ||||||||
self._time_passes_handler, buf) | ||||||||
return str(buf) | ||||||||
|
||||||||
def _dispose(self): | ||||||||
ffi.lib.LLVMPY_DisposePassBuilder(self) | ||||||||
|
||||||||
|
@@ -339,11 +381,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_CreateTimePassesHandler.restype = \ | ||||||||
ffi.LLVMTimePassesHandlerRef | ||||||||
|
||||||||
ffi.lib.LLVMPY_DisposeTimePassesHandler.argtypes = [ | ||||||||
ffi.LLVMTimePassesHandlerRef,] | ||||||||
|
||||||||
ffi.lib.LLVMPY_EnableTimePasses.argtypes = [ | ||||||||
ffi.LLVMPassBuilderRef, | ||||||||
ffi.LLVMTimePassesHandlerRef, | ||||||||
] | ||||||||
|
||||||||
ffi.lib.LLVMPY_ReportAndDisableTimePasses.argtypes = [ | ||||||||
ffi.LLVMTimePassesHandlerRef, | ||||||||
POINTER(c_char_p), | ||||||||
] | ||||||||
|
||||||||
# Pipeline builders | ||||||||
|
||||||||
ffi.lib.LLVMPY_buildPerModuleDefaultPipeline.restype = \ | ||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -3066,6 +3066,56 @@ 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.start_pass_timing() | ||||||
mpm = pb.getModulePassManager() | ||||||
mpm.run(mod, pb) | ||||||
report = pb.finish_pass_timing() | ||||||
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) | ||||||
pb.start_pass_timing() | ||||||
report = pb.finish_pass_timing() | ||||||
pb.close() | ||||||
self.assertFalse(report) | ||||||
|
||||||
def test_multiple_timers_error(self): | ||||||
mod = self.module() | ||||||
pb = self.pb() | ||||||
pb.start_pass_timing() | ||||||
mpm = pb.getModulePassManager() | ||||||
mpm.run(mod, pb) | ||||||
pb.finish_pass_timing() | ||||||
with self.assertRaises(RuntimeError): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As per suggestion below:
Suggested change
|
||||||
pb.start_pass_timing() | ||||||
pb.close() | ||||||
|
||||||
def test_empty_report_error(self): | ||||||
mod = self.module() | ||||||
pb = self.pb() | ||||||
mpm = pb.getModulePassManager() | ||||||
mpm.run(mod, pb) | ||||||
with self.assertRaises(RuntimeError): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's good to check the error is for the expected reason - for example I think
Suggested change
should do it. |
||||||
pb.finish_pass_timing() | ||||||
pb.close() | ||||||
|
||||||
|
||||||
class TestNewModulePassManager(BaseTest, NewPassManagerMixin): | ||||||
def pm(self): | ||||||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 toPassBuilder
object and is used to do stuff like this(hooks to enable/disable pass timings) after the optimisation pipeline has been built.There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
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
There was a problem hiding this comment.
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
I get an asserting failure from LLVM:
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.
There was a problem hiding this comment.
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 aRuntimeError
if it has already been called before, to protect the interpreter.There was a problem hiding this comment.
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?