This is an archive of the discontinued LLVM Phabricator instance.

[mlgo] Remove the protobuf dependency
ClosedPublic

Authored by mtrofin on Jan 13 2023, 12:29 PM.

Details

Summary

The dependency was due to the log format. This change switches to the
previously-introduced (D139370) "dependency-free" logger instead of the
protobuf-based one.

A subsequent change will clean out the unnecessary abstraction left
behind.

This change drops the logger unittest, we have sufficient test coverage
via lit tests, and a unit test would require adding, unnecesarily, a log
reader (the reader is expected to be python, for the ML side, and there
is a reader for that under Analysis/models, used for tests).

Diff Detail

Event Timeline

mtrofin created this revision.Jan 13 2023, 12:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 13 2023, 12:29 PM
mtrofin requested review of this revision.Jan 13 2023, 12:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 13 2023, 12:29 PM

LGTM
One really minor nit.

I'm assuming the performance difference is pretty minimal between this and the TF Protobuf implementation within the overall training process (although most of the overhead would probably by in parsing on the Python side I would think)?

llvm/test/CodeGen/MLRegalloc/dev-mode-extra-features-logging.ll
25–26

Maybe note here explicitly we're regexing most of the opcodes due to exact values causing test failures over minor differences? Otherwise the regex might be somewhat hard to parse without additional context.

LGTM
One really minor nit.

I'm assuming the performance difference is pretty minimal between this and the TF Protobuf implementation within the overall training process (although most of the overhead would probably by in parsing on the Python side I would think)?

Right, the main value is in removing a dependency, enable the compiler stream out observations without needing to cache them, which also means we can reuse this code to implement the interactive model evaluator for gym environments. The last 2 will happen in subsequent patches, this just achieves the first bit.

llvm/test/CodeGen/MLRegalloc/dev-mode-extra-features-logging.ll
25–26

done

Changes LGTM having read through the diff, though I'm not in any position to sign off on it :)

kazu accepted this revision.Jan 17 2023, 12:54 PM
This revision is now accepted and ready to land.Jan 17 2023, 12:54 PM
This revision was landed with ongoing or failed builds.Jan 17 2023, 1:12 PM
This revision was automatically updated to reflect the committed changes.
llvm/test/Transforms/Inline/ML/ml-test-development-mode.ll