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

[RFC][TIR] Change AllocateNode::extent to 1-D #40

Closed
wants to merge 3 commits into from

Conversation

Lunderberg
Copy link
Contributor

@Lunderberg Lunderberg commented Oct 6, 2021

This RFC changes Array<PrimExpr> AllocateNode::extents to PrimExpr AllocateNode::extent, giving the 1-d size of the buffer to be allocated. This is part of the separation between logical layout and physical layout, proposed in RFC#0039.

Rendered markdown link

@tqchen tqchen added the status: need review RFC needs review label Oct 6, 2021
@tqchen tqchen changed the title [RFC][TIR] 1-d AllocateNode::extent [RFC][TIR] Change AllocateNode::extent to 1-D Oct 6, 2021
@vinx13
Copy link
Member

vinx13 commented Oct 8, 2021

Thanks for the RFC. A quick question: RFC #39 mentioned the usage of PHYSICAL_AXIS_SEPARATOR to support n-d physical allocation (if supported by runtime), how will it work with 1-d extent here?

@Lunderberg
Copy link
Contributor Author

This is the first in a series of proposed changes, and this one on its own won't be able to support the PHYSICAL_AXIS_SEPARATOR. In the Impacted TIR Nodes section of RFC #39, under AllocateNode, it gives a description of the changes being made. This first change removes all information of the logical shape from the AllocateNode, and the follow-up change will then extend to allow for N-d physical shapes.

Lunderberg added a commit to Lunderberg/tvm that referenced this pull request Oct 22, 2021
Currently in PR at apache#9194, waiting on
RFC apache/tvm-rfcs#40.

Locally, this is a merge commit for the 'allocate_node_1d', to help me
keep track of which commits belong in which PRs.
@Lunderberg
Copy link
Contributor Author

Closing this RFC, as it is no longer applicable after significant changes made to #39.

@Lunderberg Lunderberg closed this Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: need review RFC needs review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants