This is an archive of the discontinued LLVM Phabricator instance.

[MLGO] Add support for multiple training traces per module
ClosedPublic

Authored by mtrofin on Jan 10 2022, 6:24 PM.

Details

Summary

This happens in e.g. regalloc, where we trace decisions per function,
but wouldn't want to spew N log files (i.e. one per function). So we
output a key-value association, where the key is an ID for the
sub-module object, and the value is the tensorflow::SequenceExample.

The current relation with protobuf is tenuous, so we're avoiding a
custom message type in favor of using the Struct message, but that
requires the values be wire-able strings, hence base64 encoding.

We plan on resolving the protobuf situation shortly, and improve the
encoding of such logs, but this is sufficient for now for setting up
regalloc training.

Diff Detail

Event Timeline

mtrofin created this revision.Jan 10 2022, 6:24 PM
mtrofin requested review of this revision.Jan 10 2022, 6:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 10 2022, 6:24 PM
yundiqian accepted this revision.Jan 11 2022, 1:40 PM
yundiqian added inline comments.
llvm/include/llvm/Analysis/Utils/TFUtils.h
184

from the same module in format of google::protobuf::Struct, e.g., ...

This revision is now accepted and ready to land.Jan 11 2022, 1:40 PM
snehasish added inline comments.
llvm/unittests/Analysis/TFUtilsTest.cpp
311

This should be ASSERT_TRUE since if parsing from Result fails the following EXPECT checks should not be executed.

https://testing.googleblog.com/2008/07/tott-expect-vs-assert.html

mtrofin marked an inline comment as done.Jan 11 2022, 4:06 PM
mtrofin added inline comments.
llvm/unittests/Analysis/TFUtilsTest.cpp
311

Good catch, thanks - that goes for the other tests too, I'll fix separately first.

mtrofin updated this revision to Diff 399113.Jan 11 2022, 4:12 PM
mtrofin marked an inline comment as done.

feedback

mtrofin marked an inline comment as done.Jan 11 2022, 4:13 PM
mtrofin added inline comments.
llvm/unittests/Analysis/TFUtilsTest.cpp
311

rGb7f298f17416 takes care of the others.

This revision was landed with ongoing or failed builds.Jan 11 2022, 4:27 PM
This revision was automatically updated to reflect the committed changes.