[Hexagon] Generalize builtin for Nd memory alloc with storage scope and add lowering for VTCM / Hexagon #10558

adstraw · 2022-03-10T02:15:43Z

This PR modifies the texture2d_alloca builtin --- a specialized intrinsic for allocating texture memory --- to a more general form called nd_mem_alloc_with_scope and provides a lowering for that builtin for both texture memory / OpenCL and VTCM / Hexagon.

This change allows for allocations from cache_read and cache_write schedule primitives with VTCM storage scope to be lowered to specialized interfaces in the Hexagon Device API to allocate and free VTCM: AllocVtcmWorkspace and FreeVtcmWorkspace. Where previously those allocations were lowered to AllocWorkspace and FreeWorkspace which allocate and free DDR.

This PR makes possible but does not address a standard (rather than specialized) AllocWorkspace with storage scope in the Device API.

Breakage was caused by apache#9727, which didn't account for the new `builtin::mem_copy()` when computing the stack size in `StackSizeChecker`.

… hexagon-alloc-nd-lower-vtcm

Lunderberg · 2022-03-11T18:19:06Z

src/runtime/hexagon/hexagon/hexagon_device_api_v2.cc

@@ -163,6 +170,63 @@ TVM_REGISTER_GLOBAL("device_api.hexagon.mem_copy").set_body([](TVMArgs args, TVM
  *rv = static_cast<int32_t>(0);
 });

+std::map<void*, HexagonBuffer*> vtcmallocs;
+
+TVM_REGISTER_GLOBAL("device_api.hexagon.AllocNd").set_body([](TVMArgs args, TVMRetValue* rv) {


Instead of using a lambda for the body, can we move this to a separate function? It tends to make debugging easier later on, and becomes a template for a generalized function we can propose adding to c_runtime_api.cc.

This PR creates a separate function called AllocVtcmWorkspace in the Hexagon Device API that is called from the lambda and which mirrors AllocTextureWorkspace in the OpenCL device API. These two APIs are the template for a more generalized Device API to "allocate a workspace with storage scope" but this should occur in a follow-up PR, in my opinion.

Note that the lambda must construct Device and DLDataType arguments for AllocVtcmWorkspace from primitive types passed through the builtin before making the call to AllocVtcmWorkspace.

src/runtime/hexagon/hexagon/hexagon_device_api_v2.cc

src/tir/transforms/lower_tvm_builtin.cc

csullivan

Looking great with few minor suggestions!

src/runtime/hexagon/hexagon/hexagon_device_api_v2.cc

src/tir/transforms/lower_vtcm_alloc.cc

src/tir/transforms/texture_flatten.cc

src/tir/transforms/lower_vtcm_alloc.cc

adstraw · 2022-03-14T19:35:50Z

@kparzysz-quic please let me know if you have any feedback, this is very nearly ready to merge

csullivan

LGTM!

kparzysz-quic · 2022-03-14T21:27:28Z

My only comment is about naming conventions in device_api: there is snake_case (e.g. device_api.hexagon.mem_copy), but here we're introducing CamelCase: device_api.hexagon.AllocNd. Should we have a uniform naming style?

adstraw · 2022-03-14T21:33:08Z

My only comment is about naming conventions in device_api: there is snake_case (e.g. device_api.hexagon.mem_copy), but here we're introducing CamelCase: device_api.hexagon.AllocNd. Should we have a uniform naming style?

Good point. If it's all right with you let's clean this up in the subsequent PR that will be needed to clean up the TODO items and enable nd allocations through the Hexagon Device API.

csullivan · 2022-03-15T15:14:18Z

Thanks @adstraw!

tmoreau89 · 2022-03-15T15:24:27Z

Forgot to add the usual message :)
Thank you @adstraw @csullivan @kparzysz-quic @elvin-n the PR has been merged!

…nd add lowering for VTCM / Hexagon (apache#10558) * repurpose texture flatten for vtcm; TIR lowering correct * clean up remaining code in texture flatten pass * add Alloc and FreeTexture, but failing to run over rpc * test passing with malloc in the device api * cleanup * fails in very reliable way with memory corruption * working with non-HexagonBuffer vtcm alloc * cleanup * do not pass scope through mem_copy api * [Hexagon] Resolve breakage in test_hexagon/test_cache_read_write Breakage was caused by apache#9727, which didn't account for the new `builtin::mem_copy()` when computing the stack size in `StackSizeChecker`. * use HexagonBuffer in Alloc and Free packed funcs * Added comment indicating need for StackSizeChecker::MakeMemCopy. * add AllocVtcmWorkspace and FreeVtcmWorkspace * cleanup * Updated unittests to run all contrib/test_hexagon at CI. * create separate vtcm alloc lowering pass and transform * reset texture_flatten.cc * comments * CI bump * Fix lint formatting error. * Updated fix to remove StackSizeChecker entirely. * pass device and type to device api * Bugfix, verify the precheck's allocations, not own. * Bugfix, pass context information to the precheck. * pass order and shape to device api * working * fix up types and arg passing * pass scope to device api * common builtin for texture / vtcm * add scope to freend api * format and lint * fixed missed format error * restart ci * fix test random value issue + code review feedback * fix test hang * restructure lower vtcm pass per code review feedback (option a) * format error * global.vtcm + tvm_stack_make_shape Co-authored-by: Eric Lunderberg <[email protected]>

adstraw and others added 30 commits March 2, 2022 09:53

repurpose texture flatten for vtcm; TIR lowering correct

6bbe6cc

clean up remaining code in texture flatten pass

3cb0121

add Alloc and FreeTexture, but failing to run over rpc

5365fab

test passing with malloc in the device api

8044dc0

cleanup

82f61be

fails in very reliable way with memory corruption

c1843ed

working with non-HexagonBuffer vtcm alloc

4cef769

cleanup

f13cd4c

do not pass scope through mem_copy api

e9ef946

[Hexagon] Resolve breakage in test_hexagon/test_cache_read_write

daac188

Breakage was caused by apache#9727, which didn't account for the new `builtin::mem_copy()` when computing the stack size in `StackSizeChecker`.

use HexagonBuffer in Alloc and Free packed funcs

5ca8970

Added comment indicating need for StackSizeChecker::MakeMemCopy.

8cea1e1

Merge branch 'hexagon_mem_copy_lower' into hexagon-alloc-nd-lower-vtcm

05423ea

add AllocVtcmWorkspace and FreeVtcmWorkspace

a96c062

cleanup

0ecd017

Updated unittests to run all contrib/test_hexagon at CI.

2c1ee84

create separate vtcm alloc lowering pass and transform

351b0af

reset texture_flatten.cc

0f37782

comments

6678e14

CI bump

1088c66

Fix lint formatting error.

7de3ae0

Updated fix to remove StackSizeChecker entirely.

794dbbf

Merge remote-tracking branch 'lunderberg/hexagon_mem_copy_lower' into…

c21b254

… hexagon-alloc-nd-lower-vtcm

pass device and type to device api

7b06e7c

Bugfix, verify the precheck's allocations, not own.

bc372da

Bugfix, pass context information to the precheck.

4ff6471

pass order and shape to device api

1c23651

working

7e43cd8

fix up types and arg passing

5132fd6

pass scope to device api

b28ff9c

adstraw requested review from icemelon, comaniac and zhiics as code owners March 10, 2022 02:15

fixed missed format error

caed9f1

adstraw changed the title ~~[Hexagon] Add lowering from AllocateNd to VTCM workspace allocation in Hexagon Device API~~ [Hexagon] Generalize builtin for Nd memory alloc with storage scope and add lowering for VTCM / Hexagon Mar 10, 2022

adstraw added 2 commits March 10, 2022 22:13

restart ci

0f59317

Merge branch 'main' into hexagon-alloc-nd-lower-vtcm

520f517

Lunderberg reviewed Mar 11, 2022

View reviewed changes

Lunderberg mentioned this pull request Mar 11, 2022

[Hexagon] Codegen for 2d Load/Store #10586

Merged

adstraw added 2 commits March 11, 2022 14:24

fix test random value issue + code review feedback

b7d8dd0

fix test hang

c3a3b30

csullivan requested changes Mar 12, 2022

View reviewed changes

src/runtime/hexagon/hexagon/hexagon_device_api_v2.cc Outdated Show resolved Hide resolved

src/tir/transforms/lower_vtcm_alloc.cc Outdated Show resolved Hide resolved

src/tir/transforms/texture_flatten.cc Outdated Show resolved Hide resolved

Lunderberg reviewed Mar 14, 2022

View reviewed changes

src/tir/transforms/lower_vtcm_alloc.cc Show resolved Hide resolved

adstraw added 3 commits March 14, 2022 08:45

restructure lower vtcm pass per code review feedback (option a)

53ce1ee

format error

412857c

global.vtcm + tvm_stack_make_shape

38cd975

adstraw requested review from Lunderberg and csullivan March 14, 2022 19:34

Merge branch 'main' into hexagon-alloc-nd-lower-vtcm

484cc7b

csullivan approved these changes Mar 14, 2022

View reviewed changes

kparzysz-quic approved these changes Mar 14, 2022

View reviewed changes

tmoreau89 merged commit 2f7bb58 into apache:main Mar 14, 2022

adstraw deleted the hexagon-alloc-nd-lower-vtcm branch April 6, 2022 16:31

driazati mentioned this pull request Jul 14, 2022

TVM v0.9.0.rc0 Release Candidate Notes #12102

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Hexagon] Generalize builtin for Nd memory alloc with storage scope and add lowering for VTCM / Hexagon #10558

[Hexagon] Generalize builtin for Nd memory alloc with storage scope and add lowering for VTCM / Hexagon #10558

adstraw commented Mar 10, 2022 •

edited

Loading

Lunderberg Mar 11, 2022

adstraw Mar 11, 2022 •

edited

Loading

csullivan left a comment

adstraw commented Mar 14, 2022

csullivan left a comment

kparzysz-quic commented Mar 14, 2022

adstraw commented Mar 14, 2022

csullivan commented Mar 15, 2022

tmoreau89 commented Mar 15, 2022

[Hexagon] Generalize builtin for Nd memory alloc with storage scope and add lowering for VTCM / Hexagon #10558

[Hexagon] Generalize builtin for Nd memory alloc with storage scope and add lowering for VTCM / Hexagon #10558

Conversation

adstraw commented Mar 10, 2022 • edited Loading

Lunderberg Mar 11, 2022

Choose a reason for hiding this comment

adstraw Mar 11, 2022 • edited Loading

Choose a reason for hiding this comment

csullivan left a comment

Choose a reason for hiding this comment

adstraw commented Mar 14, 2022

csullivan left a comment

Choose a reason for hiding this comment

kparzysz-quic commented Mar 14, 2022

adstraw commented Mar 14, 2022

csullivan commented Mar 15, 2022

tmoreau89 commented Mar 15, 2022

adstraw commented Mar 10, 2022 •

edited

Loading

adstraw Mar 11, 2022 •

edited

Loading