This is an archive of the discontinued LLVM Phabricator instance.

[MLInliner] In development mode, obtain the output specs from a file
ClosedPublic

Authored by mtrofin on Aug 10 2020, 11:20 AM.

Details

Summary

Different training algorithms may produce models that, besides the main
policy output (i.e. inline/don't inline), produce additional outputs
that are necessary for the next training stage. To facilitate this, in
development mode, we require the training policy infrastructure produce
a description of the outputs that are interesting to it, in the form of
a JSON file. We special-case the first entry in the JSON file as the
inlining decision - we care about its value, so we can guide inlining
during training - but treat the rest as opaque data that we just copy
over to the training log.

Diff Detail

Event Timeline

mtrofin created this revision.Aug 10 2020, 11:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2020, 11:20 AM
mtrofin requested review of this revision.Aug 10 2020, 11:20 AM
yundiqian added inline comments.Aug 16 2020, 8:46 PM
llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp
63–67

what's the motivation of providing this override alternative?

656–669

I'm a bit confused here, why we need both MUTR and Runner in the TrainingLogger? Are they just the same? (why not using ModelRunner instead of MUTR in all TrainingLogger functions?)

llvm/lib/Analysis/models/inliner/output_spec.json
1–13

where is this file being used? Is it used when running default policy?

mtrofin marked 3 inline comments as done.Aug 16 2020, 8:53 PM
mtrofin added inline comments.
llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp
63–67

Testing. See the added run in development-training-log.ll

656–669

Runner is generically typed as MLModelRunner, which could be either a ModelUnderTrainingRunner or the NoInferenceModelRunner, see lines 659 and 661.

llvm/lib/Analysis/models/inliner/output_spec.json
1–13

It is used alongside the pre-trained policy, for example in one of the current runs in tests (incl. development-training-log.ll's current test that uses that model.

yundiqian added inline comments.Aug 16 2020, 11:13 PM
llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp
656–669

ha, I see, but the APIs here looks quite confusing to me...Would it be better to add some APIs in NoInferenceModelRunner such that we don't need two different ModelRunner here?

mtrofin marked 4 inline comments as done.Aug 17 2020, 9:42 AM
mtrofin added inline comments.
llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp
656–669

you mean no-op apis to NoInferenceModelRunner? we can look into it after this CL, so we make sure the whole functionality would work in that case (and see what the usage would look like, wouldnit actually be simpler, etc)

yundiqian accepted this revision.Aug 17 2020, 1:47 PM
This revision is now accepted and ready to land.Aug 17 2020, 1:47 PM
MaskRay added inline comments.
llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp
406

Outputs.resize(MUTR->outputSpecs().size())

or

Outputs.assign(MUTR->outputSpecs().size(), std::vector<char>())

mtrofin updated this revision to Diff 286177.Aug 17 2020, 4:56 PM
mtrofin marked 2 inline comments as done.

feedback

llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp
406

yup, that's cleaner - thanks!

This revision was landed with ongoing or failed builds.Aug 17 2020, 4:57 PM
This revision was automatically updated to reflect the committed changes.