Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2/3][AOT][DeviceAPI] Add Hooks for Activate/Deactivate/Open/Close #9500

Merged
merged 2 commits into from
Nov 16, 2021

Conversation

Mousius
Copy link
Member

@Mousius Mousius commented Nov 12, 2021

This adds the relevant hooks into their starting places in the code generation. As per the C Device API RFC

This stacks on top of #9395

@Mousius Mousius changed the title [AOT][DeviceAPI] Add Hooks for Activate/Deactivate/Open/Close [2/3][AOT][DeviceAPI] Add Hooks for Activate/Deactivate/Open/Close Nov 12, 2021
@Mousius Mousius force-pushed the device-api-activate branch from fb75858 to c88bb17 Compare November 12, 2021 11:18
This adds the relevant hooks into their starting places in the code
generation. As per the [C Device API
RFC](https://github.com/apache/tvm-rfcs/blob/main/rfcs/0031-devices-api.md)
@Mousius Mousius force-pushed the device-api-activate branch from c88bb17 to 4dbe678 Compare November 13, 2021 12:47
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 @Mousius .

Few concerns around executor_factory APIs and others are basically nits.

python/tvm/relay/backend/executor_factory.py Outdated Show resolved Hide resolved
python/tvm/relay/build_module.py Show resolved Hide resolved
python/tvm/relay/build_module.py Outdated Show resolved Hide resolved
python/tvm/relay/build_module.py Outdated Show resolved Hide resolved
src/relay/backend/aot_executor_codegen.cc Show resolved Hide resolved
src/relay/backend/aot_executor_codegen.cc Show resolved Hide resolved
@Mousius Mousius force-pushed the device-api-activate branch from a69a7ac to 36042e4 Compare November 15, 2021 15:36

def representative_dataset():
for _ in range(100):
data = np.random.rand(*tuple([1, 3, 4, 3]))
Copy link
Contributor

@gromero gromero Nov 16, 2021

Choose a reason for hiding this comment

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

@Mousius Could the unpack here be directly from the list? Like: ...rand(*[1, 3, 4, 3])?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, nice spot @gromero, by the same token I don't think we even need to unpack the list? This is the same as rand(1, 3, 4,3), I'll clear this up 😸

Copy link
Contributor

Choose a reason for hiding this comment

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

@Mousius Hi! Yeah, I thought of that too (avoiding the unpack too), however I assumed you would like to keep it as [1, 3, 4, 3] just to be "more explicit" by keeping the dimensions written in a form as you pass later for example to tf.TensorSpec. Either way looks fine to me, just the form with "tuple" seems superfluous :)

Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

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

@mbs-octoml can you take a look too?

@areusch
Copy link
Contributor

areusch commented Nov 16, 2021

@manupa-arm can you take another look?

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
Copy link
Contributor

manupak commented Nov 16, 2021

Thanks @Mousius @areusch .

Lets follow up with any outstanding comments in 3/3. @gromero @mbs-octoml

@manupak manupak merged commit 948641c into apache:main Nov 16, 2021
@Mousius Mousius deleted the device-api-activate branch November 16, 2021 09:38
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Dec 1, 2021
…pache#9500)

* [AOT][DeviceAPI] Add Hooks for Activate/Deactivate/Open/Close

This adds the relevant hooks into their starting places in the code
generation. As per the [C Device API
RFC](https://github.com/apache/tvm-rfcs/blob/main/rfcs/0031-devices-api.md)

* Standardise on `lowered_ir_mods` and correct device_hook variable name
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Dec 1, 2021
…pache#9500)

* [AOT][DeviceAPI] Add Hooks for Activate/Deactivate/Open/Close

This adds the relevant hooks into their starting places in the code
generation. As per the [C Device API
RFC](https://github.com/apache/tvm-rfcs/blob/main/rfcs/0031-devices-api.md)

* Standardise on `lowered_ir_mods` and correct device_hook variable name
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
…pache#9500)

* [AOT][DeviceAPI] Add Hooks for Activate/Deactivate/Open/Close

This adds the relevant hooks into their starting places in the code
generation. As per the [C Device API
RFC](https://github.com/apache/tvm-rfcs/blob/main/rfcs/0031-devices-api.md)

* Standardise on `lowered_ir_mods` and correct device_hook variable name
yangulei pushed a commit to yangulei/tvm that referenced this pull request Jan 11, 2022
…pache#9500)

* [AOT][DeviceAPI] Add Hooks for Activate/Deactivate/Open/Close

This adds the relevant hooks into their starting places in the code
generation. As per the [C Device API
RFC](https://github.com/apache/tvm-rfcs/blob/main/rfcs/0031-devices-api.md)

* Standardise on `lowered_ir_mods` and correct device_hook variable name
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
…pache#9500)

* [AOT][DeviceAPI] Add Hooks for Activate/Deactivate/Open/Close

This adds the relevant hooks into their starting places in the code
generation. As per the [C Device API
RFC](https://github.com/apache/tvm-rfcs/blob/main/rfcs/0031-devices-api.md)

* Standardise on `lowered_ir_mods` and correct device_hook variable name
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