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

[M2a] Storage align #422

Merged
merged 11 commits into from
Aug 9, 2021
Merged

[M2a] Storage align #422

merged 11 commits into from
Aug 9, 2021

Conversation

vinx13
Copy link
Collaborator

@vinx13 vinx13 commented Jul 31, 2021

No description provided.

@tqchen
Copy link
Contributor

tqchen commented Aug 2, 2021

cc @Hzfengsy please help to shepherd this PR

Comment on lines 511 to 513

########## Schedule: loop binding/annotation ##########
def storage_align(self, block: BlockRV, buffer_index: int, axis: int, factor: int,
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's a bit strange to put storage_align() in the "loop binding/annotation" category, as the primitive doesn't play with any loop. My proposal is to add a new category named "buffer transformation", which can contain primitives like storage_align(), set_scope(), etc.

Copy link
Member

Choose a reason for hiding this comment

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

+1

src/tir/schedule/concrete_schedule.cc Outdated Show resolved Hide resolved
src/tir/schedule/concrete_schedule.h Outdated Show resolved Hide resolved
src/tir/schedule/primitive/bind_annotate.cc Outdated Show resolved Hide resolved
src/tir/schedule/schedule.cc Outdated Show resolved Hide resolved
tests/python/unittest/test_tir_schedule_storage_align.py Outdated Show resolved Hide resolved
Comment on lines 511 to 513

########## Schedule: loop binding/annotation ##########
def storage_align(self, block: BlockRV, buffer_index: int, axis: int, factor: int,
Copy link
Member

Choose a reason for hiding this comment

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

+1

src/tir/schedule/primitive/bind_annotate.cc Outdated Show resolved Hide resolved
src/tir/schedule/primitive/bind_annotate.cc Outdated Show resolved Hide resolved
src/tir/schedule/primitive/bind_annotate.cc Outdated Show resolved Hide resolved
@vinx13 vinx13 force-pushed the staging_storage_align branch 2 times, most recently from 327627f to 2912342 Compare August 5, 2021 22:54
src/tir/schedule/analysis.h Outdated Show resolved Hide resolved
src/tir/schedule/primitive/block_annotate.cc Show resolved Hide resolved
python/tvm/tir/schedule/schedule.py Show resolved Hide resolved
src/tir/schedule/analysis.h Outdated Show resolved Hide resolved
src/tir/schedule/analysis/analysis.cc Outdated Show resolved Hide resolved
src/tir/schedule/analysis/analysis.cc Outdated Show resolved Hide resolved
src/tir/schedule/analysis/analysis.cc Outdated Show resolved Hide resolved
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.

LGTM if you can address current comments and check the lint.

@junrushao
Copy link
Member

Please address the comments and send the PR to mainline. I'd love to do another round of review there

Copy link
Collaborator

@MasterJH5574 MasterJH5574 left a comment

Choose a reason for hiding this comment

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

LGTM. I think we can merge it and send it to mainline.

src/tir/schedule/analysis/analysis.cc Outdated Show resolved Hide resolved
vinx13 and others added 3 commits August 9, 2021 12:48
Co-authored-by: Ruihang Lai <lairuihangdongdong@qq.com>
Co-authored-by: Ruihang Lai <lairuihangdongdong@qq.com>
@vinx13
Copy link
Collaborator Author

vinx13 commented Aug 9, 2021

Comments have been addressed, please review again

@junrushao
Copy link
Member

Please send this PR to mainline for a second review :-)

@vinx13 vinx13 merged commit 95805ae into tlc-pack:staging Aug 9, 2021
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.

5 participants