This is an archive of the discontinued LLVM Phabricator instance.

[NFC][TFUtils] Extract out the output spec loader
ClosedPublic

Authored by mtrofin on Nov 18 2020, 4:18 PM.

Details

Summary

It's generic for the 'development mode', not specific to the inliner
case.

Diff Detail

Event Timeline

mtrofin created this revision.Nov 18 2020, 4:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 18 2020, 4:18 PM
mtrofin requested review of this revision.Nov 18 2020, 4:18 PM
yundiqian added inline comments.Nov 18 2020, 5:35 PM
llvm/include/llvm/Analysis/Utils/TFUtils.h
204–207

Do we still need this function?

llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp
486

why not passing in OutputSpecs?

mtrofin marked 2 inline comments as done.Nov 18 2020, 5:49 PM
mtrofin added inline comments.
llvm/include/llvm/Analysis/Utils/TFUtils.h
204–207

We can keep it as a lower primitive.

llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp
486

because those are LoggedFeatureSpecs, so they include the extra name for logging. The evaluator doesn't care about that, and we shouldn't muddy the abstraction layers.

yundiqian added inline comments.Nov 18 2020, 6:05 PM
llvm/include/llvm/Analysis/Utils/TFUtils.h
204–207

Is it a higher primitive? This function calls TFModelEvalutor(..., function_ref<TensorSpec(size_t)> GetOutputSpecs,, ...)

llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp
486

How about making a copy into std::vector<TensorSpec&> outputspec? It would make the code much easier

mtrofin marked 4 inline comments as done.Nov 18 2020, 6:11 PM
mtrofin added inline comments.
llvm/include/llvm/Analysis/Utils/TFUtils.h
204–207

sorry, I misread what you were referring to - the ctor. Tests use it. You are correct, it's a higher primitive.

llvm/lib/Analysis/DevelopmentModeInlineAdvisor.cpp
486

because I'd prefer avoiding copies, if possible.

yundiqian accepted this revision.Nov 18 2020, 6:15 PM
This revision is now accepted and ready to land.Nov 18 2020, 6:15 PM
This revision was landed with ongoing or failed builds.Nov 18 2020, 8:18 PM
This revision was automatically updated to reflect the committed changes.
mtrofin marked 2 inline comments as done.