[USMP] Adding support for U1 usecase for constant pools by d-smirnov · Pull Request #10189 · apache/tvm

d-smirnov · 2022-02-08T11:49:26Z

areusch

apologies for the delay @manupa-arm @d-smirnov. broadly i think this is ok, my main question is around the purpose WorkspacePoolInfo class. also a few cleanups around the PR.

include/tvm/ir/memory_pools.h

include/tvm/tir/usmp/utils.h

src/ir/memory_pools.cc

src/runtime/aot_executor/aot_executor.cc

src/target/source/source_module.cc

src/tir/usmp/analysis/extract_buffer_info.cc

tests/python/relay/aot/test_cpp_aot.py

lhutton1

Thanks @d-smirnov, I took a look based on a naive understanding of this work. Overall LGTM modulo a couple of nit's and @areusch's outstanding comments. As @manupa-arm mentioned, perhaps most of these comments we can take in a followup so it doesn't keep getting conflicted?

include/tvm/runtime/metadata_types.h

python/tvm/ir/memory_pools.py

src/target/llvm/codegen_cpu.cc

src/target/source/source_module.cc

lhutton1 · 2022-06-15T10:13:11Z

include/tvm/tir/usmp/utils.h

+/*
+ * \brief The ConstantInfoNode contains numeric literal in RO pool
+ */
+struct ConstantInfoNode : public Object {


Did this one get done?

lhutton1 · 2022-06-15T10:22:39Z

src/tir/usmp/transform/convert_pool_allocations_to_offsets.cc

+        AllocateConst allocate_const_node = Downcast<AllocateConst>(kv.first);
+        extent_size = CalculateExtentsSize(allocate_const_node.operator->());
+      } else {
+        LOG(FATAL) << "Not supported";


Think we missed this one

tests/python/unittest/test_tir_usmp_analysis_extract_bufferinfo.py

python/tvm/relay/build_module.py

tests/python/relay/aot/test_cpp_aot.py

areusch

thanks @d-smirnov ! i think this is about ready. it would be great if we could address the Python inheritance question. if we're worried about conflicts, we could land this PR and fix in a follow-on, but it would be great if we could agree on a fix here before landing

include/tvm/ir/memory_pools.h

python/tvm/ir/memory_pools.py

src/ir/memory_pools.cc

src/target/source/source_module.cc

Constants are now aggregated into one struct and initialized in default_lib0.c file Change-Id: I34d61f8139c8a92c06944fe990ba892a660476fd Unit test fixed Change-Id: I436e7b6d6b3064b3f8bbfbb048d4296b63a6b69c

Addressed: * PoolInfo splitted to WorkspacePoolInfo and ConstantPoolInfo * workspace_byte_alignment moved to ExecutorCodegenMetadata * getModuleAlignment -> GetModuleAlignment * GenerateInternalWorkspaceBuffers refactored * reverted format change of src/tir/transforms/legalize_packed_calls.cc * addressed comments for src/tir/usmp/analysis/extract_buffer_info.cc * removed commented code from include/tvm/tir/usmp/utils.h Change-Id: I7d1b32884b0e5992e2e00c7838c85e425d9c25fd

Change-Id: I573a05fa1cb4037ae83691f7dff2c2724b1d7700

Added ConstantMemoryPools Change-Id: If1e391c631575980564bca790ba33748c82d907f

Change-Id: Iacc7a9d734a505dfa0d8d32d23ea3f57e6de8582

added constant_alignment unit tests updated Change-Id: I378193cb9e675e352c61d96ff4e09655090053e1

Change-Id: Ia4411d59c4a376c01326fed366cdb196a432899e

Change-Id: Ia2077bdeb1d2c6c9827eeef90ab410ae31b8c4a4

renamed pools and consts to workspace_pools and constant_pools

manupak · 2022-06-20T08:45:02Z

Try to catch up with remaining concerns here and it seems the inheritance question was the only thing ?

For that,
Is the resolution reached/proposed here is that creation (via init) of PoolInfo object is encapsulated away from python side?

@areusch @d-smirnov

manupak · 2022-06-21T10:27:07Z

My view is to keep the base class as it provides the right abstraction to the internals that does not need to differentiate between whether a constant pool or a workspace pool -- thus agreeing with current approach by @d-smirnov. In the same time, I also dont see a reason to create generic PoolInfo objects straight away -- thus disabling the construction of that might solve this concerns raised by @areusch unless we missed something in the conversation ?

Looking forward to progress on this a bit sooner...

areusch

sorry for the delay @manupa-arm. i agree disabling the python-side constructor does fix the problem. and we do have other cases where IR base classes exist, so let's merge this then.

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

* [TIR.Constant] U1 usecase Constants are now aggregated into one struct and initialized in default_lib0.c file Change-Id: I34d61f8139c8a92c06944fe990ba892a660476fd Unit test fixed Change-Id: I436e7b6d6b3064b3f8bbfbb048d4296b63a6b69c * Refactored Addressed: * PoolInfo splitted to WorkspacePoolInfo and ConstantPoolInfo * workspace_byte_alignment moved to ExecutorCodegenMetadata * getModuleAlignment -> GetModuleAlignment * GenerateInternalWorkspaceBuffers refactored * reverted format change of src/tir/transforms/legalize_packed_calls.cc * addressed comments for src/tir/usmp/analysis/extract_buffer_info.cc * removed commented code from include/tvm/tir/usmp/utils.h Change-Id: I7d1b32884b0e5992e2e00c7838c85e425d9c25fd * more unit test fixes Change-Id: I573a05fa1cb4037ae83691f7dff2c2724b1d7700 * More refactoring and unit test fixes Added ConstantMemoryPools Change-Id: If1e391c631575980564bca790ba33748c82d907f * bugfix Change-Id: Iacc7a9d734a505dfa0d8d32d23ea3f57e6de8582 * refactoring. added constant_alignment added constant_alignment unit tests updated Change-Id: I378193cb9e675e352c61d96ff4e09655090053e1 * unit-test bugix Change-Id: Ia4411d59c4a376c01326fed366cdb196a432899e * unit test fix Change-Id: Ia2077bdeb1d2c6c9827eeef90ab410ae31b8c4a4 * Added support for c++ runtime * refactored * renamed pools and consts renamed pools and consts to workspace_pools and constant_pools * addressed upstream comments * addressed upstream comments-2 * addressed upstream comments-3

* [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 #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 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

d-smirnov force-pushed the U1-usecase branch from 3bcd7b2 to ede9805 Compare February 8, 2022 12:20

manupak changed the title ~~U1 usecase~~ [USMP] Adding support for U1 usecase for constant pools Feb 8, 2022

manupak mentioned this pull request Feb 8, 2022

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

Closed

21 tasks

d-smirnov force-pushed the U1-usecase branch 3 times, most recently from 40d2923 to abe8567 Compare February 17, 2022 22:40

d-smirnov force-pushed the U1-usecase branch 3 times, most recently from 985686d to c9911e2 Compare March 9, 2022 13:46

d-smirnov marked this pull request as ready for review March 9, 2022 23:03

d-smirnov requested review from Hzfengsy, MarisaKirisame, ZihengJiang, anijain2305, areusch, comaniac, icemelon, jroesch, junrushao, jwfromm, kparzysz-quic, masahi, mbrookhart, merrymercy, slyubomirsky, tqchen, vinx13, wweic, yzhliu and zhiics as code owners March 9, 2022 23:03

areusch requested changes Jun 14, 2022

View reviewed changes

lhutton1 approved these changes Jun 15, 2022

View reviewed changes

areusch reviewed Jun 16, 2022

View reviewed changes

d-smirnov force-pushed the U1-usecase branch 2 times, most recently from 7b1a5f6 to 243b1f9 Compare June 17, 2022 16:02

d-smirnov and others added 14 commits June 19, 2022 22:02

[TIR.Constant] U1 usecase

2f5cf7d

Constants are now aggregated into one struct and initialized in default_lib0.c file Change-Id: I34d61f8139c8a92c06944fe990ba892a660476fd Unit test fixed Change-Id: I436e7b6d6b3064b3f8bbfbb048d4296b63a6b69c

more unit test fixes

a2de6a7

Change-Id: I573a05fa1cb4037ae83691f7dff2c2724b1d7700

More refactoring and unit test fixes

fb022b4

Added ConstantMemoryPools Change-Id: If1e391c631575980564bca790ba33748c82d907f

bugfix

40a6d35

Change-Id: Iacc7a9d734a505dfa0d8d32d23ea3f57e6de8582

refactoring. added constant_alignment

c6151f6

added constant_alignment unit tests updated Change-Id: I378193cb9e675e352c61d96ff4e09655090053e1

unit-test bugix

ec37636

Change-Id: Ia4411d59c4a376c01326fed366cdb196a432899e

unit test fix

32bfb7c

Change-Id: Ia2077bdeb1d2c6c9827eeef90ab410ae31b8c4a4

Added support for c++ runtime

3984d76

refactored

750deb7

renamed pools and consts

944f2bf

renamed pools and consts to workspace_pools and constant_pools

addressed upstream comments

fd192f2

addressed upstream comments-2

092011d

addressed upstream comments-3

a102a5b

d-smirnov force-pushed the U1-usecase branch from 243b1f9 to a102a5b Compare June 19, 2022 21:03

areusch approved these changes Jun 22, 2022

View reviewed changes

areusch merged commit c80da03 into apache:main Jun 22, 2022

dchauhan-arm added a commit to dchauhan-arm/tvm that referenced this pull request Jul 6, 2022

[TVMC] Workspace Pools Parameters

3c72359

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

driazati mentioned this pull request Jul 14, 2022

TVM v0.9.0.rc0 Release Candidate Notes #12102

Closed

Conversation

d-smirnov commented Feb 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

areusch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lhutton1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lhutton1 Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

lhutton1 Jun 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

areusch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

manupak commented Jun 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

manupak commented Jun 21, 2022

Uh oh!

areusch left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

d-smirnov commented Feb 8, 2022 •

edited

Loading

manupak commented Jun 20, 2022 •

edited

Loading