Skip to content

Fix incorrect AOT Memory Planning - #8926

Merged
jroesch merged 1 commit into
apache:mainfrom
Mousius:aot-memory-planning
Sep 7, 2021
Merged

Fix incorrect AOT Memory Planning#8926
jroesch merged 1 commit into
apache:mainfrom
Mousius:aot-memory-planning

Conversation

@Mousius

Copy link
Copy Markdown
Member

This change introduces a second memory planning phase in the AOT code
generator once the storage rewrite pass has been completed, fixing
incorrectly sized workspaces for a variety of models.

It comes with accompanying tests so we can safely refactor this later.

Also corrected a typo in the TE compiler regards the memory alignment
argument 😸

Co-authored-by: Manupa Karunaratne Manupa.Karunaratne@arm.com

@Mousius

Copy link
Copy Markdown
MemberAuthor

CC @manupa-arm

@Mousius

Copy link
Copy Markdown
MemberAuthor

Also CC @mbs-octoml@electriclilies for reviews 😸

@Mousius
Mousiusforce-pushed the aot-memory-planning branch 3 times, most recently from 4fbb9f5 to cc0c43bCompareSeptember 3, 2021 18:51
Comment threadsrc/relay/backend/aot_executor_codegen.cc

@electriclilieselectriclilies left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall, LGTM! I'd like to see CalculateWorkspaceBytes in a pass in the future instead of being run directly in the exectutor, but this is fine for now

Comment threadsrc/relay/backend/aot_executor_codegen.cc
@Mousius
Mousiusforce-pushed the aot-memory-planning branch 3 times, most recently from aea2a68 to 97401c5CompareSeptember 6, 2021 08:51
This change introduces a second memory planning phase in the AOT code
generator once the storage rewrite pass has been completed, fixing
incorrectly sized workspaces for a variety of models.
It comes with accompanying tests so we can safely refactor this later.
Also corrected a typo in the TE compiler regards the memory alignment
argument 😸
Co-authored-by: Manupa Karunaratne <Manupa.Karunaratne@arm.com>
@jroesch

Copy link
Copy Markdown
Member

Overall, LGTM! I'd like to see CalculateWorkspaceBytes in a pass in the future instead of being run directly in the exectutor, but this is fine for now

@Mousius is it possible for you guys to put this as a backlog item? will land this version but in general better to try and split these things into passes so we can make the executor code gens function more uniform instead of each one having different arguments/return values, etc.

@jroesch
jroesch merged commit 53a3cba into apache:mainSep 7, 2021
ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
This change introduces a second memory planning phase in the AOT code
generator once the storage rewrite pass has been completed, fixing
incorrectly sized workspaces for a variety of models.
It comes with accompanying tests so we can safely refactor this later.
Also corrected a typo in the TE compiler regards the memory alignment
argument 😸
Co-authored-by: Manupa Karunaratne <Manupa.Karunaratne@arm.com>
Co-authored-by: Manupa Karunaratne <Manupa.Karunaratne@arm.com>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
This change introduces a second memory planning phase in the AOT code
generator once the storage rewrite pass has been completed, fixing
incorrectly sized workspaces for a variety of models.
It comes with accompanying tests so we can safely refactor this later.
Also corrected a typo in the TE compiler regards the memory alignment
argument 😸
Co-authored-by: Manupa Karunaratne <Manupa.Karunaratne@arm.com>
Co-authored-by: Manupa Karunaratne <Manupa.Karunaratne@arm.com>
Sign up for freeto 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

@Mousius@jroesch@electriclilies@manupak@mbs-octoml