Skip to content

Conversation

@lhutton1
Copy link
Contributor

Adds a pass called HoistAllocates to move allocate nodes to the top of the body of the main function. In doing so, it opens the door to other optimizations that need to swap the ordering of external calls.

Pass illustration:
(before)

allocate {
    extern_call(...)
    allocate {
        extern_call(...)
    }
}

(after)

allocate {
    allocate {
        extern_call(...)
        extern_call(...)
    }
}

cc @manupa-arm @NicolaLancellotti @ekalda @GeorgeARM

@github-actions github-actions bot requested a review from manupak March 23, 2022 10:26
Copy link
Contributor

@manupak manupak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lhutton1 ! for this backend pass.

I did an initial pass.

Copy link
Contributor Author

@lhutton1 lhutton1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review @manupa-arm, I'll respond here since they are related :) Since this pass runs in _lower_to_tir, the pass will only ever see individual prim funcs with the "main" global function. The refactor (#10599) didn't cover this part of the compiler as it would have made it much more complex, so all the passes here: https://github.com/apache/tvm/blob/main/python/tvm/relay/backend/contrib/ethosu/tir/compiler.py#L76 are still function -> function, while the module -> module part is orchestrated by the LowerToTIR pass. We could add the functionality, although it won't be needed until we choose to refactor this part of the lowering to be module->module as well

@manupak
Copy link
Contributor

manupak commented Mar 23, 2022

Thanks @lhutton1 ! I see.

It might be worth mentioning in the docs for the pass that this pass is only meant to be run in LowerToTIR() composite pass.
We might needs some checks to that effect with tests -- i.e. checking whether the module has a single PrimFunc called "main" and if not throwing a message that the pass is only supposed to run in LowerToTIR() where the above requirement is satisfied.

@github-actions github-actions bot requested a review from manupak March 24, 2022 11:38
Copy link
Contributor

@NicolaLancellotti NicolaLancellotti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@ekalda ekalda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work @lhutton1!

Copy link
Contributor

@manupak manupak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM bar @ekalda's comments.

Adds a pass called `HoistAllocates` to move allocate nodes to the top
of the body of the main function. In doing so, it opens the door to
other optimizations that need to swap the ordering of external calls.

Pass illustration:
(before)
```
allocate {
    extern_call {
        allocate {
            extern_call {

            }
        }
    }
}
```

(after)
```
allocate {
    allocate {
        extern_call
        extern_call
    }
}
```

Change-Id: Ibcfc3c75b15deebb5c6645a4923a6ddf683b37c4
* uses prim func pass, rather than module pass.
* adds error message informing user to run this pass with LowerToTIR()
  pass for now.

Change-Id: I57757b9dc5bff0208034a974a341c09cce0294bc
With a test to back this case up.

Change-Id: I670809f5ee53b583a15d9b783852dda3089756e9
Change-Id: I3e9f24adfe992ace4e03238a18a8378b03257e1a
Copy link
Contributor

@manupak manupak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@manupak manupak merged commit 079eb4e into apache:main Mar 25, 2022
@manupak
Copy link
Contributor

manupak commented Mar 25, 2022

Thanks! @lhutton1 @ekalda @NicolaLancellotti

@lhutton1 lhutton1 deleted the hoist-allocates branch March 25, 2022 20:34
pfk-beta pushed a commit to pfk-beta/tvm that referenced this pull request Apr 11, 2022
…he#10725)

* [microNPU] Add a pass to move allocate nodes to the outer scope

Adds a pass called `HoistAllocates` to move allocate nodes to the top
of the body of the main function. In doing so, it opens the door to
other optimizations that need to swap the ordering of external calls.

Pass illustration:
(before)
```
allocate {
    extern_call {
        allocate {
            extern_call {

            }
        }
    }
}
```

(after)
```
allocate {
    allocate {
        extern_call
        extern_call
    }
}
```

Change-Id: Ibcfc3c75b15deebb5c6645a4923a6ddf683b37c4

* address comments

* uses prim func pass, rather than module pass.
* adds error message informing user to run this pass with LowerToTIR()
  pass for now.

Change-Id: I57757b9dc5bff0208034a974a341c09cce0294bc

* Support allocates when not followed by a sequence statement

With a test to back this case up.

Change-Id: I670809f5ee53b583a15d9b783852dda3089756e9

* Add new directory tir/contrib/ethosu to cmake build

Change-Id: I3e9f24adfe992ace4e03238a18a8378b03257e1a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants