[PATCH] D142168: [mlgo] Stream the training data

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: llvm/lib/CodeGen/MLRegallocEvictAdvisor.cpp:468-469
+    if (Log->currentContext() != MF.getName()) {
+      MF.getFunction().getContext().emitError(
+          "The training log context shouldn't have had changed.");
+    }
----------------
MatzeB wrote:
> MatzeB wrote:
> > mtrofin wrote:
> > > MatzeB wrote:
> > > > I think I am hitting this for cases where `RegAllocGreedy` bails out early in the `if (!hasVirtRegAlloc()) return false;` check and never initializes the advisor.
> > > > 
> > > > Replacing with `Log->switchCurrentContext()` for my experiments...
> > > ack. I'll take a look. Probably in that case `(!hasVirtRegAlloc())` there's also no output to log, right?
> > Guess I can also try to first test for `hasObservationInProgress()` and bail out on that before checking the function name... Rebuilding and testing right now...
> checking for `hasObservationInProgress()` first was not enough to avoid hitting those errors. (the "InProgress" part of the function name seems somewhat confusing anyway as this seems to be more of an "everHadAnObservation()" behavior rather than being in the middle of a startObservation() / endObservation() thingy...)
I'm not sure it's really that early-exit in RAGreedy that makes me hit that error, but I am hitting it for multiple functions within many compilation units here; will continue digging next week...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142168/new/

https://reviews.llvm.org/D142168