[JIT] Add Exit Transform / Convert To SSA to docs #24114
Conversation
eellison
requested review from
Krovatkin,
ZolotukhinM,
driazati,
suo and
zdevito
and removed request for
suo
torch/csrc/jit/docs/OVERVIEW.md
Outdated
torch/csrc/jit/docs/OVERVIEW.md
Outdated
torch/csrc/jit/docs/OVERVIEW.md
Outdated
There was a problem hiding this comment.
Could you please add a high level overview of why we're doing this (in contrast to "what")? It might be not obvious why this all is necessary and how it relates to classic SSA. For instance, in the conventional definition of SSA the central part is phi-nodes. We don't have them because we work only with structured control flow, but we never mentioned that here. But that's what explains all of what's happening here: we're trying to represent constructs with breaks/continues/returns in a form of well-structured CFG that only have loops with no early exits and ifs.
Another thing that might be confusing is the load/store part - we're not loading or storing anything there, it's just a trick we use to figure out input/output values of a block, and I think it's also worth mentioning somewhere.
torch/csrc/jit/docs/OVERVIEW.md
Outdated
There was a problem hiding this comment.
Nit: "Once we hit an Exit Node" sounds a bit strange to me as we don't execute anything here. Maybe rephrase it a bit (like "if a block has an exit node, further instructions will never be executed").
torch/csrc/jit/docs/OVERVIEW.md
Outdated
There was a problem hiding this comment.
this seems to be a little bit too dry. How are we tracking in-scope values? Are we doing something similar to what we used to do in compiler.cpp before? Ideally, we could use a simple example that shows a graph with loads and stores and then the one w/o them right before we remove prim::LoopContinuations.
torch/csrc/jit/docs/OVERVIEW.md
Outdated
There was a problem hiding this comment.
continue_loop is inserted at the end of every inner block?
There was a problem hiding this comment.
@eellison has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Pull Request resolved: pytorch#24114 Differential Revision: D19780828 Pulled By: eellison fbshipit-source-id: d481ad886b2ad6349a1646672e507336d45759fb
6 participants
No description provided.