materialize icon indicating copy to clipboard operation
materialize copied to clipboard

Regions around LIR operators

Open frankmcsherry opened this issue 2 years ago • 0 comments

This PR adds regions around all non-primitive LIR operators. Some operators are less clear that they should be wrapped, e.g. Let and Get, and some turn in to a single operator and so a whole region didn't seem all that helpful (e.g. negate which is a single map_in_place operator).

The PR also cleans up various uses of regions, attempting to thread errors through regions that already exist, and to use enter_region() and leave_region() in place of enter() and leave().

It is not clear that we should do this, as it introduces a bit of noise and runtime overhead into the works. It might make sense, for example, to put regions on any subgraphs of unbounded size (e.g. Reduce and Join(Linear), which are not currently wrapped in regions in main).

Example new hierarchical view:

Screen Shot 2022-09-20 at 3 58 19 PM

The non-hierarchical visualizer doesn't seem to report the dataflow as existing; not sure what is up with that, otherwise I would show it looking worse (because it doesn't grok the regions as operators and gets tangled up with them).

Motivation

  • This PR adds a maybe-desirable feature.

Tips for reviewer

Ignore whitespace to reduce the diff size substantially (the regions change indentation levels throughout).

Checklist

  • [ ] This PR has adequate test coverage / QA involvement has been duly considered.

  • [ ] This PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way) and therefore is tagged with a T-protobuf label.

  • [x] This PR includes the following user-facing behavior changes:

    • The hierarchical memory visualizer gets more hierarchical structure, and the non-hierarchical visualizer becomes more unreadable.

frankmcsherry avatar Sep 19 '22 20:09 frankmcsherry