[TVMC] Workspace Pools Parameters #11427

dchauhan-arm · 2022-05-24T07:55:20Z

Attributes from tvmc are now passable into the created PoolInfo objects
inside WorkspaceMemoryPools. This is passed in to relay.build that get
attached to IRModule attribute.

cc @Mousius @gromero

Attributes from tvmc are now passable into the created PoolInfo objects inside WorkspaceMemoryPools. This is passed in to relay.build that get attached to IRModule attribute.

dchauhan-arm · 2022-05-24T07:56:15Z

cc @leandron @manupa-arm @areusch

Mousius

Nice work @dchauhan-arm, just took a quick pass but will take a deeper look later 😸

tests/python/driver/tvmc/test_command_line.py

Address comments, fix linting. Testing improved. Change-Id: Iea79329b6b9ec1cbc51e5c293449bf6dd43b00c5

Change-Id: Ib0ef594a04d3a0ac0e901e856ee73a37809db880

Update workspace pools test naming Change-Id: Ib698d6248be1e6f44340f27db3641c985bc5c5d8

areusch

thanks @dchauhan-arm , a couple questions. don't take these as prescriptive, just wondering if we have thoughts in those directions right now

areusch · 2022-06-02T20:19:48Z

python/tvm/driver/tvmc/workspace_pools.py

+
+def workspace_pools_recombobulate(parsed, targets):
+    """Reconstructs the Workspace Pools args and returns a WorkspaceMemoryPool object"""
+    WORKSPACE_POOL_PARAMS = [


could we mark where this list comes from and include "keep in sync with" comments in both places?

If they get out of sync, I agree with the comment you left below that PoolInfo should complain as that's what tvmc is wrapping

areusch · 2022-06-02T20:20:57Z

python/tvm/driver/tvmc/workspace_pools.py

+
+    return WorkspaceMemoryPools(
+        [
+            PoolInfo(


alternatively if this thing complained when a parameters was missing or invalidly named, that would be ok as a way to resolve my earlier comment

I agree with this approach, if PoolInfo complained about an incorrect parameter being supplied then TVMC should complain as well

I think we still need the keep-in-sync comment because PoolInfo could be added with arguments with defaults and tvmc support for that could be missed out.

PoolInfo has documented properties

PoolInfo will get upset if you pass incorrect properties

tvmc arguments are documented

I don't really see the need to add an additional comment to link these together? I don't think we do this for the tuning parameters either: https://github.com/apache/tvm/blob/main/python/tvm/driver/tvmc/autotuner.py ?

My concern was PoolInfo could be augmented with new parameters with defaults and we could easily miss those out in the tvmc layer. Having the comment will help the author and the reviewer to not to miss them out in the tvmc layer.

Oh we only need the keep-in-sync comment at the PoolInfo side and not here, because "PoolInfo will get upset if you pass incorrect properties".

python/tvm/driver/tvmc/workspace_pools.py

areusch · 2022-06-10T00:18:36Z

@dchauhan-arm mention me when you're ready for another look here.

manupak

Broadly looking good!
Only comment from my side would be, is there a test to check whether pool parameter overrides are working ? (I might've missed it)

manupak · 2022-06-22T08:34:28Z

python/tvm/driver/tvmc/workspace_pools.py

+
+    return WorkspaceMemoryPools(
+        [
+            PoolInfo(


I think we still need the keep-in-sync comment because PoolInfo could be added with arguments with defaults and tvmc support for that could be missed out.

Add test for parameter overrides. Change-Id: I67d5470dcfbfbc9ab27f34e20a9269d2070193ca

manupak · 2022-06-29T16:20:01Z

@tvm-bot rerun

Rebasing over apache#10189 Updates to the way a WorkspaceMemoryPool object is created Change-Id: I1f0e1d240343af311ddb3ed5c564cc1ab329f463

manupak · 2022-07-07T16:49:44Z

@tvm-bot rerun

Fix linting, fix CI Change-Id: If75f8709ac4ad925655eca54b3e5c1bb09d025e8

Add mcpu and mattr to target registry for cmsis-nn Change-Id: I15257b8d01624c071c738cab6d12ecb84ed6cb16

manupak

Thanks @dchauhan-arm !

Only a small thing -- we need to make sure single pool overrides work.

tests/python/driver/tvmc/test_workspace_pools.py

Added test for override on single pool when multiple pools are present Updated functionality of parsing multiple attributes Change-Id: I2c0745051b7a923dd7f75040bfb89bbc99376a11

Mousius

This LGTM, @manupa-arm can you take another look?

manupak

LGTM!

manupak · 2022-07-21T09:32:50Z

@leandron as a codeowner, could you take a final look ?

leandron

LGTM, thanks @dchauhan-arm!

After apache#11427, `tvmc compile` wouldn't work for external codegens that don't have a `Target` registered by `TVM_REGISTER_TARGET_KIND`. Such external codegens can be expected to have no workspace pools and may not always have a target associated as their implementation predates this mechanism. While it is likely a `Target` is specified for all external codegens in the future, we should still support external codegens without an associated `Target` until this is enforced. Change-Id: Ida8bf85ed1cafd301b9465641c66a5370d73c429

After #11427, `tvmc compile` wouldn't work for external codegens that don't have a `Target` registered by `TVM_REGISTER_TARGET_KIND`. Such external codegens can be expected to have no workspace pools and may not always have a target associated as their implementation predates this mechanism. While it is likely a `Target` is specified for all external codegens in the future, we should still support external codegens without an associated `Target` until this is enforced. Co-authored-by: Chris Sidebottom <chris.sidebottom@arm.com>

* [TVMC] Workspace Pools Parameters Attributes from tvmc are now passable into the created PoolInfo objects inside WorkspaceMemoryPools. This is passed in to relay.build that get attached to IRModule attribute. * [TVMC] Workspace Pools Parameters Address comments, fix linting. Testing improved. Change-Id: Iea79329b6b9ec1cbc51e5c293449bf6dd43b00c5 * [TVMC] Workspace Pools Parameters Update workspace pools test naming Change-Id: Ib698d6248be1e6f44340f27db3641c985bc5c5d8 * [TVMC] Workspace Pools Parameters Add test for parameter overrides. Change-Id: I67d5470dcfbfbc9ab27f34e20a9269d2070193ca * [TVMC] Workspace Pools Parameters Rebasing over apache#10189 Updates to the way a WorkspaceMemoryPool object is created Change-Id: I1f0e1d240343af311ddb3ed5c564cc1ab329f463 * [TVMC] Workspace Pools Parameters Fix linting, fix CI Change-Id: If75f8709ac4ad925655eca54b3e5c1bb09d025e8 * [TVMC] Workspace Pools Parameters Add mcpu and mattr to target registry for cmsis-nn Change-Id: I15257b8d01624c071c738cab6d12ecb84ed6cb16 * [TVMC] Workspace Pools Parameters Added test for override on single pool when multiple pools are present Updated functionality of parsing multiple attributes Change-Id: I2c0745051b7a923dd7f75040bfb89bbc99376a11

…#12253) After apache#11427, `tvmc compile` wouldn't work for external codegens that don't have a `Target` registered by `TVM_REGISTER_TARGET_KIND`. Such external codegens can be expected to have no workspace pools and may not always have a target associated as their implementation predates this mechanism. While it is likely a `Target` is specified for all external codegens in the future, we should still support external codegens without an associated `Target` until this is enforced. Co-authored-by: Chris Sidebottom <chris.sidebottom@arm.com>

* [TVMC] Workspace Pools Parameters Attributes from tvmc are now passable into the created PoolInfo objects inside WorkspaceMemoryPools. This is passed in to relay.build that get attached to IRModule attribute. * [TVMC] Workspace Pools Parameters Address comments, fix linting. Testing improved. Change-Id: Iea79329b6b9ec1cbc51e5c293449bf6dd43b00c5 * [TVMC] Workspace Pools Parameters Update workspace pools test naming Change-Id: Ib698d6248be1e6f44340f27db3641c985bc5c5d8 * [TVMC] Workspace Pools Parameters Add test for parameter overrides. Change-Id: I67d5470dcfbfbc9ab27f34e20a9269d2070193ca * [TVMC] Workspace Pools Parameters Rebasing over apache#10189 Updates to the way a WorkspaceMemoryPool object is created Change-Id: I1f0e1d240343af311ddb3ed5c564cc1ab329f463 * [TVMC] Workspace Pools Parameters Fix linting, fix CI Change-Id: If75f8709ac4ad925655eca54b3e5c1bb09d025e8 * [TVMC] Workspace Pools Parameters Add mcpu and mattr to target registry for cmsis-nn Change-Id: I15257b8d01624c071c738cab6d12ecb84ed6cb16 * [TVMC] Workspace Pools Parameters Added test for override on single pool when multiple pools are present Updated functionality of parsing multiple attributes Change-Id: I2c0745051b7a923dd7f75040bfb89bbc99376a11

[TVMC] Workspace Pools Parameters

a325bd4

Attributes from tvmc are now passable into the created PoolInfo objects inside WorkspaceMemoryPools. This is passed in to relay.build that get attached to IRModule attribute.

Mousius requested changes May 24, 2022

View reviewed changes

tests/python/driver/tvmc/test_command_line.py Outdated Show resolved Hide resolved

tests/python/driver/tvmc/test_command_line.py Outdated Show resolved Hide resolved

tests/python/driver/tvmc/test_command_line.py Outdated Show resolved Hide resolved

dchauhan-arm and others added 2 commits May 24, 2022 15:32

Merge branch 'apache:main' into pers-params

4d95fbe

[TVMC] Workspace Pools Parameters

2f3ca98

Address comments, fix linting. Testing improved. Change-Id: Iea79329b6b9ec1cbc51e5c293449bf6dd43b00c5

github-actions bot requested a review from gromero May 24, 2022 14:33

Merge branch 'apache:main' into pers-params

8f9d23a

Change-Id: Ib0ef594a04d3a0ac0e901e856ee73a37809db880

dchauhan-arm force-pushed the pers-params branch from c7b29b5 to 8f9d23a Compare May 25, 2022 12:59

dchauhan-arm and others added 4 commits May 26, 2022 09:37

Merge branch 'apache:main' into pers-params

88174c1

Merge branch 'apache:main' into pers-params

18b3cd8

[TVMC] Workspace Pools Parameters

08693a2

Update workspace pools test naming Change-Id: Ib698d6248be1e6f44340f27db3641c985bc5c5d8

Merge branch 'apache:main' into pers-params

6740398

areusch reviewed Jun 2, 2022

View reviewed changes

Merge branch 'main' into pers-params

5b367a6

manupak reviewed Jun 22, 2022

View reviewed changes

[TVMC] Workspace Pools Parameters

31cc299

Add test for parameter overrides. Change-Id: I67d5470dcfbfbc9ab27f34e20a9269d2070193ca

dchauhan-arm force-pushed the pers-params branch from 8a7f9db to 31cc299 Compare June 22, 2022 15:53

manupak added the status: need update need update based on feedbacks label Jun 29, 2022

Merge branch 'apache:main' into pers-params

e58c330

manupak mentioned this pull request Jun 30, 2022

[RFC] Unified Static Memory Planning (USMP) Tracking Issue #8404

Closed

21 tasks

dchauhan-arm and others added 2 commits July 6, 2022 15:00

[TVMC] Workspace Pools Parameters

3c72359

Rebasing over apache#10189 Updates to the way a WorkspaceMemoryPool object is created Change-Id: I1f0e1d240343af311ddb3ed5c564cc1ab329f463

Merge branch 'apache:main' into pers-params

d74d034

dchauhan-arm added 2 commits July 8, 2022 10:55

[TVMC] Workspace Pools Parameters

f5237df

Fix linting, fix CI Change-Id: If75f8709ac4ad925655eca54b3e5c1bb09d025e8

[TVMC] Workspace Pools Parameters

d4f6aa6

Add mcpu and mattr to target registry for cmsis-nn Change-Id: I15257b8d01624c071c738cab6d12ecb84ed6cb16

manupak requested changes Jul 11, 2022

View reviewed changes

tests/python/driver/tvmc/test_workspace_pools.py Show resolved Hide resolved

[TVMC] Workspace Pools Parameters

3e107d7

Added test for override on single pool when multiple pools are present Updated functionality of parsing multiple attributes Change-Id: I2c0745051b7a923dd7f75040bfb89bbc99376a11

Mousius approved these changes Jul 19, 2022

View reviewed changes

manupak approved these changes Jul 21, 2022

View reviewed changes

manupak added status: need review and removed status: need update need update based on feedbacks labels Jul 25, 2022

leandron approved these changes Jul 25, 2022

View reviewed changes

leandron merged commit 75ec1cf into apache:main Jul 25, 2022

dchauhan-arm deleted the pers-params branch July 25, 2022 16:30

lhutton1 mentioned this pull request Aug 1, 2022

[TVMC] Only load extra targets when there are workspace pools #12253

Merged

AndrewZhaoLuo mentioned this pull request Oct 4, 2022

TVM v0.10.0.rc0 Release Candidate Notes #12979

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TVMC] Workspace Pools Parameters #11427

[TVMC] Workspace Pools Parameters #11427

dchauhan-arm commented May 24, 2022 •

edited by github-actions bot

Loading

dchauhan-arm commented May 24, 2022

Mousius left a comment

areusch left a comment

areusch Jun 2, 2022

dchauhan-arm Jun 7, 2022

areusch Jun 2, 2022

dchauhan-arm Jun 7, 2022

manupak Jun 22, 2022 •

edited

Loading

Mousius Jun 22, 2022 •

edited

Loading

manupak Jun 22, 2022

manupak Jun 23, 2022

areusch commented Jun 10, 2022

manupak left a comment •

edited

Loading

manupak Jun 22, 2022 •

edited

Loading

manupak commented Jun 29, 2022

manupak commented Jul 7, 2022

manupak left a comment

Mousius left a comment

manupak left a comment

manupak commented Jul 21, 2022 •

edited

Loading

leandron left a comment

[TVMC] Workspace Pools Parameters #11427

[TVMC] Workspace Pools Parameters #11427

Conversation

dchauhan-arm commented May 24, 2022 • edited by github-actions bot Loading

dchauhan-arm commented May 24, 2022

Mousius left a comment

Choose a reason for hiding this comment

areusch left a comment

Choose a reason for hiding this comment

areusch Jun 2, 2022

Choose a reason for hiding this comment

dchauhan-arm Jun 7, 2022

Choose a reason for hiding this comment

areusch Jun 2, 2022

Choose a reason for hiding this comment

dchauhan-arm Jun 7, 2022

Choose a reason for hiding this comment

manupak Jun 22, 2022 • edited Loading

Choose a reason for hiding this comment

Mousius Jun 22, 2022 • edited Loading

Choose a reason for hiding this comment

manupak Jun 22, 2022

Choose a reason for hiding this comment

manupak Jun 23, 2022

Choose a reason for hiding this comment

areusch commented Jun 10, 2022

manupak left a comment • edited Loading

Choose a reason for hiding this comment

manupak Jun 22, 2022 • edited Loading

Choose a reason for hiding this comment

manupak commented Jun 29, 2022

manupak commented Jul 7, 2022

manupak left a comment

Choose a reason for hiding this comment

Mousius left a comment

Choose a reason for hiding this comment

manupak left a comment

Choose a reason for hiding this comment

manupak commented Jul 21, 2022 • edited Loading

leandron left a comment

Choose a reason for hiding this comment

dchauhan-arm commented May 24, 2022 •

edited by github-actions bot

Loading

manupak Jun 22, 2022 •

edited

Loading

Mousius Jun 22, 2022 •

edited

Loading

manupak left a comment •

edited

Loading

manupak Jun 22, 2022 •

edited

Loading

manupak commented Jul 21, 2022 •

edited

Loading