Skip to content

Use AddBufferDonor to replace SetUpAlias for input output aliasing#6769

Merged
JackCaoG merged 1 commit intomasterfrom
JackCaoG/new_aliasing
Mar 20, 2024
Merged

Use AddBufferDonor to replace SetUpAlias for input output aliasing#6769
JackCaoG merged 1 commit intomasterfrom
JackCaoG/new_aliasing

Conversation

@JackCaoG
Copy link
Copy Markdown
Collaborator

Existing SetUpAlias requires us(PyTorch/XLA) to specify the input and output index we want to alias. This can be difficult for the SPMD use case because we don't know the output sharding hence we can't be sure if input and output will have the same buffer size.

AddBufferDonor is a newish API added by XLA team. It only requires us to give compiler a list of input buffer that can be reused for the output. It is up to the compiler to determine how to reuse these buffer. I have been user AddBufferDonor for the manual buffer donation for dynamo. In this pr I extend the use case to the auto-aliasing in LTC(the way it works is to determine all of the input buffers that does not have a XLATensor associated and conclude an inplace operation happened so the buffer can be reused).

@JackCaoG JackCaoG requested a review from yeounoh March 20, 2024 21:48
@JackCaoG JackCaoG marked this pull request as ready for review March 20, 2024 21:48
@@ -1261,59 +1260,14 @@ XLAGraphExecutor::BuildInputOutputAliases(
// this buffer is not needed after execution since XLATensor will get a
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.

This is the case we reuse / donate?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yea, the idea is we don't care which output will this input get aliased to. It is up to the compiler.

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.

Makes sense, we are reusing instead of creating a new one.

Copy link
Copy Markdown
Contributor

@yeounoh yeounoh left a comment

Choose a reason for hiding this comment

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

LGTM

@JackCaoG
Copy link
Copy Markdown
Collaborator Author

Let's see if TPUCI will pass before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants