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.

Sign up for GitHub

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

Jump to bottom

Adding annotations for tir.allocate #9168

Merged
merged 2 commits into from
Oct 7, 2021
Merged

Adding annotations for tir.allocate #9168

merged 2 commits into from
Oct 7, 2021

Conversation

manupak
Copy link
Contributor

@manupak manupak commented Sep 30, 2021

This commit is adding annotations for tir.allocate node to be used as hints for future transformations.

Please refer to the discussion : apache/tvm-rfcs#23

cc: @tqchen @junrushao1994 @areusch

Mousius
Mousius reviewed Sep 30, 2021
View reviewed changes
python/tvm/tir/stmt.py Outdated Show resolved Hide resolved
@manupak manupak force-pushed the tir_allocate_annotatations branch from 9081089 to 506c2f2 Compare October 1, 2021 11:13
Mousius
Mousius approved these changes Oct 1, 2021
View reviewed changes
Copy link
Member

@Mousius Mousius left a comment

Choose a reason for hiding this comment

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

Makes sense to me 😸

@manupak manupak changed the title Adding annotation for tir.allocate Adding annotations for tir.allocate Oct 2, 2021
@manupak manupak force-pushed the tir_allocate_annotatations branch from 8e4a870 to f4f916d Compare October 4, 2021 14:36
@areusch
Copy link
Contributor

areusch commented Oct 4, 2021

please tag the tracking issue @manupa-arm

areusch
areusch reviewed Oct 4, 2021
View reviewed changes
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.

deferring to @vinx13 for approval but lgtm

vinx13
vinx13 requested changes Oct 5, 2021
View reviewed changes
Copy link
Member

@vinx13 vinx13 left a comment

Choose a reason for hiding this comment

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

Some minor comments, otherwise LGTM

include/tvm/tir/stmt.h Outdated Show resolved Hide resolved
include/tvm/tir/stmt.h Outdated Show resolved Hide resolved
@manupak
Adding annotation for tir.allocate
4971d09 
This commit is adding annotations for tir.allocate
node to be used as hints for future transformations.

Change-Id: I02a3a875c38c3edd449385da5b741ef4958bb47f
@manupak manupak force-pushed the tir_allocate_annotatations branch from f4f916d to 4971d09 Compare October 5, 2021 10:19
@manupak
Copy link
Contributor Author

manupak commented Oct 5, 2021

Thanks @vinx13 .
I have addressed them now, PTAL.

@manupak manupak mentioned this pull request Oct 5, 2021
Merged
Hzfengsy
Hzfengsy requested changes Oct 5, 2021
View reviewed changes
Copy link
Member

@Hzfengsy Hzfengsy left a comment

Choose a reason for hiding this comment

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

Please also update the Script syntax and add a testcase.

The related codes are here:

tvm/python/tvm/script/tir/scope_handler.py

Lines 106 to 115 in d9a5ff5

class Allocate(WithScopeHandler):
"""With scope handler T.allocate(extents, dtype, scope, condition)"""
def __init__(self):
def allocate(extents, dtype, scope, condition=True, span=None):
condition = tvm.runtime.convert(condition)
scope = tvm.runtime.convert(scope)
return tvm.tir.Allocate(
self.buffer_var, dtype, extents, condition, self.body, span=span
)

@Hzfengsy
Copy link
Member

Hzfengsy commented Oct 5, 2021

Please also update the three printers:

tvm/src/tir/ir/stmt.cc

Line 385 in d9a5ff5

.set_dispatch<AllocateNode>([](const ObjectRef& node, ReprPrinter* p) {

tvm/src/printer/tvmscript_printer.cc

Line 762 in d9a5ff5

Doc TVMScriptPrinter::VisitStmt_(const AllocateNode* op) {

tvm/src/printer/tir_text_printer.cc

Line 449 in d9a5ff5

Doc TIRTextPrinter::VisitStmt_(const AllocateNode* op) {

@manupak manupak requested a review from zhiics as a code owner October 6, 2021 09:54
@manupak
Copy link
Contributor Author

manupak commented Oct 6, 2021

@Hzfengsy thanks for the review!

I think I've done the necessary changes now. PTAL.

@manupak manupak mentioned this pull request Oct 6, 2021
Closed
21 tasks
Hzfengsy
Hzfengsy approved these changes Oct 6, 2021
View reviewed changes
Copy link
Member

@Hzfengsy Hzfengsy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. Thanks, @manupa-arm

@manupak manupak force-pushed the tir_allocate_annotatations branch from 00d2218 to 31d8db9 Compare October 6, 2021 17:58
@manupak manupak mentioned this pull request Oct 6, 2021
Merged
@manupak manupak force-pushed the tir_allocate_annotatations branch from 31d8db9 to 5ff4f7f Compare October 6, 2021 22:28
@manupak
Adding annotation for tir.allocate
94b7435 
* adding tvmscript support
* adding tir text printing support

Change-Id: Id0b6725b2e79c23f6b8ff192772f1ea4125a27c2
@manupak manupak force-pushed the tir_allocate_annotatations branch from 5ff4f7f to 94b7435 Compare October 7, 2021 05:35
@manupak
Copy link
Contributor Author

manupak commented Oct 7, 2021

@vinx13 @Hzfengsy merge?

@Hzfengsy Hzfengsy merged commit 2dae303 into apache:main Oct 7, 2021
@Hzfengsy
Copy link
Member

Hzfengsy commented Oct 7, 2021

Sorry for the late response. Thanks @manupa-arm.

ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
@manupak @ylc
Adding annotations for tir.allocate (apache#9168)
b2dbafd 
* Adding annotation for tir.allocate

This commit is adding annotations for tir.allocate
node to be used as hints for future transformations.

Change-Id: I02a3a875c38c3edd449385da5b741ef4958bb47f

* Adding annotation for tir.allocate

* adding tvmscript support
* adding tir text printing support

Change-Id: Id0b6725b2e79c23f6b8ff192772f1ea4125a27c2
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
@manupak @ylc
Adding annotations for tir.allocate (apache#9168)
d9348ee 
* Adding annotation for tir.allocate

This commit is adding annotations for tir.allocate
node to be used as hints for future transformations.

Change-Id: I02a3a875c38c3edd449385da5b741ef4958bb47f

* Adding annotation for tir.allocate

* adding tvmscript support
* adding tir text printing support

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

@areusch areusch areusch left review comments

@Mousius Mousius Mousius approved these changes

@vinx13 vinx13 vinx13 approved these changes

@Hzfengsy Hzfengsy Hzfengsy approved these changes

@comaniac comaniac Awaiting requested review from comaniac

@jroesch jroesch Awaiting requested review from jroesch

@junrushao junrushao Awaiting requested review from junrushao

@kparzysz-quic kparzysz-quic Awaiting requested review from kparzysz-quic

@masahi masahi Awaiting requested review from masahi

@merrymercy merrymercy Awaiting requested review from merrymercy

@tqchen tqchen Awaiting requested review from tqchen

@yzhliu yzhliu Awaiting requested review from yzhliu

@ZihengJiang ZihengJiang Awaiting requested review from ZihengJiang

@zhiics zhiics Awaiting requested review from zhiics

Assignees

@vinx13 vinx13

Labels
status: need review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@manupak @areusch @Hzfengsy @Mousius @vinx13