lhutton1 commented on code in PR #15349:
URL: https://github.com/apache/tvm/pull/15349#discussion_r1268133213
##########
tests/python/driver/tvmc/test_command_line.py:
##########
@@ -272,3 +272,20 @@ def test_tvmc_logger_set_basicConfig(monkeypatch,
tmpdir_factory, keras_simple):
_main(compile_args)
mock_basicConfig.assert_called_with(stream=sys.stdout)
+
+
+def test_tvmc_print_pass_times(capsys, keras_simple, tmpdir_factory):
+ pytest.importorskip("tensorflow")
+ tmpdir = tmpdir_factory.mktemp("out")
+ print_cmd = "--print-pass-times"
+
+ # Compile model
+ module_file = os.path.join(tmpdir, "keras-tvm.tar")
+ compile_cmd = f"tvmc compile --target 'llvm' {keras_simple} --output
{module_file} {print_cmd}"
+ compile_args = compile_cmd.split(" ")[1:]
+ _main(compile_args)
+
+ # Check for timing results output
+ expected_out = "Printing results of timing profile..."
+ captured_out = capsys.readouterr().out
+ assert expected_out in captured_out
Review Comment:
We should also try to check the pass breakdown exists and that it makes some
form of sense. Could we check for some known parts of the outputs like pass
names and the timing units?
##########
python/tvm/driver/tvmc/compiler.py:
##########
@@ -356,6 +363,13 @@ def compile_model(
if codegen["config_key"] is not None:
config[codegen["config_key"]] = codegen_from_cli["opts"]
+ timing_inst = None
Review Comment:
`compile_model` is also used by the TVMC Python API, so if a user adds
`PassTimingInstrument` to their list of instruments and calls `compile_model`
providing the list of instruments, they may get an unexpected output printed to
the console.
Since it is relatively easy for a python API user to add the instrument and
print it themselves, I think it would be better to move the printing
functionality up to `drive_compile` so that the feature is only exposed to the
command line API. It would also simplify checking if the option was enabled.
Happy to hear more thoughts from others though.
##########
python/tvm/driver/tvmc/compiler.py:
##########
@@ -442,6 +456,11 @@ def compile_model(
if dumps:
save_dumps(package_path, dumps)
+ # Print compilation times per pass
+ if timing_inst:
+ print("Printing results of timing profile...")
Review Comment:
Perhaps we could rename to "Compilation time breakdown by pass:" - I feel
like it's a bit more informative
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
